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

[Steve French]

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 AM Bjoern Doebel <doebel@xxxxxxxxx> wrote:
>
> 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 ownership.")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Bjoern Doebel <doebel@xxxxxxxxx>
> 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 char *path, __u64 *pnmode,
>                                 cifs_put_tlink(tlink);
>                                 return rc;
>                         }
> -                       if (mode_from_sid)
> -                               nsecdesclen +=
> -                                       le16_to_cpu(dacl_ptr->num_aces) * sizeof(struct smb_ace);
> -                       else /* cifsacl */
> -                               nsecdesclen += le16_to_cpu(dacl_ptr->size);
> +                       /*
> +                        * Worst case: every ACE is rewritten with a new SID of
> +                        * SID_MAX_SUB_AUTHORITIES sub-auths -> sizeof(smb_ace) each,
> +                        * plus the smb_acl header replace_sids_and_copy_aces() emits.
> +                        */
> +                       nsecdesclen += sizeof(struct smb_acl) +
> +                               le16_to_cpu(dacl_ptr->num_aces) * sizeof(struct smb_ace);
>                 }
>         }
>
> --
> 2.50.1
>
>


-- 
Thanks,

Steve