Re: [PATCH] ksmbd: restore DACL size on check_add_overflow() to avoid malformed ACL
From: Namjae Jeon
Date: Thu Jul 02 2026 - 22:31:29 EST
On Thu, Jul 2, 2026 at 8:33 PM Wentao Guan <guanwentao@xxxxxxxxxxxxx> 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: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Wentao Guan <guanwentao@xxxxxxxxxxxxx>
Applied it to #ksmbd-for-next-next.
Thanks!