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!
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.