Re: [PATCH] ksmbd: restore DACL size on check_add_overflow() to avoid malformed ACL
Namjae Jeon <[email protected]>
| Newsgroups | org.kernel.vger.linux-cifs,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <CAKYAXd9dgEhagjzofg_H4mx1kYg=tFEE49RrwU9WY-xPMoEVZw@mail.gmail.com> |
On Thu, Jul 2, 2026 at 8:33 PM Wentao Guan <[email protected]> wrote: > > check_add_overflow() unconditionally writes the truncated sum into *d > even on overflow, per its contract in include/linux/overflow.h. > The four check_add_overflow() guards in set_posix_acl_entries_dacl() > and set_ntacl_dacl() break out of the ACE-building loops on overflow, > but the truncated *size is then consumed downstream at the end of > set_ntacl_dacl(): > > pndacl->size = cpu_to_le16(le16_to_cpu(pndacl->size) + size); > > This produces an on-wire NT ACL whose pndacl->size under-reports the > bytes actually written by the preceding fill_ace_for_sid()/memcpy() > calls, yielding a malformed ACL that can trigger out-of-bounds reads > when re-parsed by clients or ksmbd itself. > > Restore *size to its pre-addition value on each overflow branch (via > `*size -= ace_sz` / `size -= nt_ace_size`) so that after the break, > *size once again holds the cumulative size of the successfully-written > ACEs. The committed ACL is then truncated-but-self-consistent rather > than malformed. > > The ksmbd DACL builders are the only check_add_overflow() sites found > where an overflow path breaks out of a loop and the destination value > is consumed afterward. The other nearby break-style cases either > return -EINVAL on overflow (transport_ipc.c) or break without > consuming the overflowed destination value afterward (buildid.c). > > Assisted-by: atomcode:glm-5.2 > Assisted-by: Codex:gpt-5.5 > > Fixes: 299f962c0b02 ("ksmbd: use check_add_overflow() to prevent u16 DACL size overflow") > Cc: [email protected] > Signed-off-by: Wentao Guan <[email protected]> Applied it to #ksmbd-for-next-next. Thanks!