Re: [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl()
Bjoern Doebel <[email protected]> Fri, 10 Jul 2026 06:37:41 +0000
| Newsgroups | gmane.linux.kernel.stable,gmane.linux.kernel.cifs,gmane.linux.kernel,gmane.network.samba.internals |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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: [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 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