* [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl()
@ 2026-07-09 15:54 Bjoern Doebel
2026-07-09 22:15 ` Steve French
0 siblings, 1 reply; 3+ messages in thread
From: Bjoern Doebel @ 2026-07-09 15:54 UTC (permalink / raw)
To: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, linux-kernel, samba-technical
Cc: stable, doebel, nmanthey
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@vger.kernel.org
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl()
2026-07-09 15:54 [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl() Bjoern Doebel
@ 2026-07-09 22:15 ` Steve French
2026-07-10 6:37 ` Bjoern Doebel
0 siblings, 1 reply; 3+ messages in thread
From: Steve French @ 2026-07-09 22:15 UTC (permalink / raw)
To: Bjoern Doebel
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, linux-kernel, samba-technical,
stable, nmanthey
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@amazon.de> 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@vger.kernel.org
> Signed-off-by: Bjoern Doebel <doebel@amazon.de>
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl()
2026-07-09 22:15 ` Steve French
@ 2026-07-10 6:37 ` Bjoern Doebel
0 siblings, 0 replies; 3+ messages in thread
From: Bjoern Doebel @ 2026-07-10 6:37 UTC (permalink / raw)
To: Steve French
Cc: Bjoern Doebel, Steve French, Paulo Alcantara, Ronnie Sahlberg,
Shyam Prasad N, Tom Talpey, Bharath SM, linux-cifs, linux-kernel,
samba-technical, stable, nmanthey
Hi Steve,
On Thu, Jul 09, 2026 at 05:15:40PM -0500, Steve French wrote:
> 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?
I shared my reproducer script via PM.
This triggered the following KASAN splat in testing (and no longer does
with the patch applied):
[ 46.251162] ==================================================================
[ 46.278880] BUG: KASAN: slab-out-of-bounds in build_sec_desc.constprop.0+0x3010/0x3a70 [cifs]
[ 46.319912] Write of size 4 at addr ffff888155556374 by task chown/3439
[ 46.351546]
[ 46.358846] CPU: 4 UID: 0 PID: 3439 Comm: chown Not tainted 7.1.0-rc6+ #35 PREEMPT(full)
[ 46.358850] Hardware name: Amazon EC2 c6i.4xlarge/, BIOS 1.0 10/16/2017
[ 46.358853] Call Trace:
[ 46.358856] <TASK>
[ 46.358859] dump_stack_lvl+0x51/0x70
[ 46.358866] print_address_description.constprop.0+0x2c/0x3a0
[ 46.358871] ? build_sec_desc.constprop.0+0x3010/0x3a70 [cifs]
[ 46.358941] print_report+0xb4/0x270
[ 46.358944] ? kasan_addr_to_slab+0x9/0x70
[ 46.358948] kasan_report+0xb4/0xe0
[ 46.358951] ? build_sec_desc.constprop.0+0x3010/0x3a70 [cifs]
[ 46.359015] build_sec_desc.constprop.0+0x3010/0x3a70 [cifs]
[ 46.359077] ? queue_folios_pte_range+0x45c/0x7a0
[ 46.359082] ? __pfx_build_sec_desc.constprop.0+0x10/0x10 [cifs]
[ 46.359143] ? __find_readable_file+0x310/0x540 [cifs]
[ 46.359220] ? kasan_save_track+0x10/0x30
[ 46.359222] ? __kasan_kmalloc+0x7b/0x90
[ 46.359226] id_mode_to_cifs_acl+0x31c/0x760 [cifs]
[ 46.359295] ? __pfx_id_mode_to_cifs_acl+0x10/0x10 [cifs]
[ 46.359356] ? __build_path_from_dentry_optional_prefix+0x176/0x620 [cifs]
[ 46.359432] cifs_setattr_nounix+0xc5b/0x1860 [cifs]
[ 46.359505] ? __pfx_cifs_setattr_nounix+0x10/0x10 [cifs]
[ 46.359573] ? __pfx___vfs_getxattr+0x10/0x10
[ 46.359577] ? __pfx_current_time+0x10/0x10
[ 46.359580] cifs_setattr+0x173/0x2b0 [cifs]
[ 46.359648] notify_change+0x832/0xf20
[ 46.359651] ? __pfx_from_vfsuid+0x10/0x10
[ 46.359655] ? chown_common+0x422/0x5e0
[ 46.359658] chown_common+0x422/0x5e0
[ 46.359662] ? __pfx_chown_common+0x10/0x10
[ 46.359665] ? check_heap_object+0x6f/0x490
[ 46.359669] ? strncpy_from_user+0x3b/0x1f0
[ 46.359673] do_fchownat+0x124/0x160
[ 46.359677] ? __pfx_do_fchownat+0x10/0x10
[ 46.359680] __x64_sys_fchownat+0xb9/0x150
[ 46.359684] ? arch_exit_to_user_mode_prepare.constprop.0+0x95/0xc0
[ 46.359688] do_syscall_64+0xaf/0x550
[ 46.359693] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 46.359695] RIP: 0033:0x7fd413b00b0e
[ 46.359709] Code: 48 8b 0d ed b2 0f 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 04 01 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ba b2 0f 00 f7 d8 64 89 01 48
[ 46.359712] RSP: 002b:00007ffc3eb973d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000104
[ 46.359715] RAX: ffffffffffffffda RBX: 00007ffc3eb97700 RCX: 00007fd413b00b0e
[ 46.359717] RDX: 0000000000001770 RSI: 000055e6afa81740 RDI: 00000000ffffff9c
[ 46.359719] RBP: 000055e6afa7fb70 R08: 0000000000000000 R09: 0000000000000000
[ 46.359720] R10: 00000000ffffffff R11: 0000000000000246 R12: 000055e6afa81740
[ 46.359722] R13: 0000000000000001 R14: 00000000ffffff9c R15: 000055e6afa7fb00
[ 46.359725] </TASK>
[ 46.359726]
[ 47.532861] Allocated by task 3439:
[ 47.549557] kasan_save_stack+0x20/0x40
[ 47.567957] kasan_save_track+0x10/0x30
[ 47.586353] __kasan_kmalloc+0x7b/0x90
[ 47.604316] __kmalloc_noprof+0x1c1/0x530
[ 47.623566] id_mode_to_cifs_acl+0x2d0/0x760 [cifs]
[ 47.646748] cifs_setattr_nounix+0xc5b/0x1860 [cifs]
[ 47.670768] cifs_setattr+0x173/0x2b0 [cifs]
[ 47.690945] notify_change+0x832/0xf20
[ 47.708918] chown_common+0x422/0x5e0
[ 47.726468] do_fchownat+0x124/0x160
[ 47.743582] __x64_sys_fchownat+0xb9/0x150
[ 47.763261] do_syscall_64+0xaf/0x550
[ 47.780816] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 47.804785]
[ 47.812163] The buggy address belongs to the object at ffff888155556000
[ 47.812163] which belongs to the cache kmalloc-1k of size 1024
[ 47.872006] The buggy address is located 0 bytes to the right of
[ 47.872006] allocated 884-byte region [ffff888155556000, ffff888155556374)
[ 47.934359]
[ 47.941237] The buggy address belongs to the physical page:
[ 47.967735] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x155550
[ 48.006191] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 48.042968] flags: 0x17ffffc0000040(head|node=0|zone=2|lastcpupid=0x1fffff)
[ 48.076404] page_type: f5(slab)
[ 48.091399] raw: 0017ffffc0000040 ffff888100042dc0 dead000000000100 dead000000000122
[ 48.128595] raw: 0000000000000000 0000000800100010 00000000f5000000 0000000000000000
[ 48.165814] head: 0017ffffc0000040 ffff888100042dc0 dead000000000100 dead000000000122
[ 48.203005] head: 0000000000000000 0000000800100010 00000000f5000000 0000000000000000
[ 48.240615] head: 0017ffffc0000003 fffffffffffffe01 00000000ffffffff 00000000ffffffff
[ 48.278257] head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
[ 48.315854] page dumped because: kasan: bad access detected
[ 48.342361]
[ 48.349658] Memory state around the buggy address:
[ 48.372332] ffff888155556200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 48.407026] ffff888155556280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 48.441656] >ffff888155556300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 04 fc
[ 48.476272] ^
[ 48.509238] ffff888155556380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 48.543873] ffff888155556400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 48.578508] ==================================================================
Bjoern
> On Thu, Jul 9, 2026 at 10:55 AM Bjoern Doebel <doebel@amazon.de> 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@vger.kernel.org
> > Signed-off-by: Bjoern Doebel <doebel@amazon.de>
> > 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-10 6:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 15:54 [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl() Bjoern Doebel
2026-07-09 22:15 ` Steve French
2026-07-10 6:37 ` Bjoern Doebel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox