Re: [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl()

Steve French <[email protected]> Thu, 9 Jul 2026 17:15:40 -0500
Newsgroups gmane.linux.kernel.stable,gmane.linux.kernel.cifs,gmane.linux.kernel,gmane.network.samba.internals
Message-ID <CAH2r5mt_Pd=wd-pXdaYkNpWE9NfeM0bhnEy-LKuhgOZchVPTJA@mail.gmail.com>
When I tried this it changed the length used (for chown with cifsacl
mount option) from 88 bytes to 236 bytes
which seems suspicious.  Have you been able to reproduce the bug this
patch is supposed to fix?

On Thu, Jul 9, 2026 at 10:55=E2=80=AFAM Bjoern Doebel <[email protected]> wr=
ote:
>
> Budget the destination buffer for the worst case in both branches:
> every rewritten ACE may take sizeof(struct smb_ace) bytes (which
> already accounts for an smb_sid with SID_MAX_SUB_AUTHORITIES
> sub-authorities), plus the smb_acl header that
> replace_sids_and_copy_aces() emits.
>
> Fixes: bc3e9dd9d104 ("cifs: Change SIDs in ACEs while transferring file o=
wnership.")
> Cc: [email protected]
> Signed-off-by: Bjoern Doebel <[email protected]>
> Assisted-by: Kiro:claude-opus-4.6
> ---
>  fs/smb/client/cifsacl.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
> index 07cf0e5782337..6d572dd995d79 100644
> --- a/fs/smb/client/cifsacl.c
> +++ b/fs/smb/client/cifsacl.c
> @@ -1812,11 +1812,13 @@ id_mode_to_cifs_acl(struct inode *inode, const ch=
ar *path, __u64 *pnmode,
>                                 cifs_put_tlink(tlink);
>                                 return rc;
>                         }
> -                       if (mode_from_sid)
> -                               nsecdesclen +=3D
> -                                       le16_to_cpu(dacl_ptr->num_aces) *=
 sizeof(struct smb_ace);
> -                       else /* cifsacl */
> -                               nsecdesclen +=3D le16_to_cpu(dacl_ptr->si=
ze);
> +                       /*
> +                        * Worst case: every ACE is rewritten with a new =
SID of
> +                        * SID_MAX_SUB_AUTHORITIES sub-auths -> sizeof(sm=
b_ace) each,
> +                        * plus the smb_acl header replace_sids_and_copy_=
aces() emits.
> +                        */
> +                       nsecdesclen +=3D sizeof(struct smb_acl) +
> +                               le16_to_cpu(dacl_ptr->num_aces) * sizeof(=
struct smb_ace);
>                 }
>         }
>
> --
> 2.50.1
>
>


--=20
Thanks,

Steve