[PATCH] smb: client: bound dirent name against end of SMB response in cifs_filldir

[PATCH] smb: client: bound dirent name against end of SMB response in cifs_filldir

[Jay Vadayath]

cifs_filldir() copies the entry name out of an SMB1 TRANS2_FIND_FIRST /
FIND_NEXT response using a length (de.namelen) supplied by the server.
The kmalloc'd SMB response buffer is bounded, but nothing checks that
de.name + de.namelen still lies inside that buffer before the eventual
filldir64() -> verify_dirent_name() -> memchr() reads namelen bytes.

A hostile SMB1 server that returns an oversized FileNameLength in a
directory entry therefore causes memchr() to read past the end of the
response slab buffer. Reachable from any user who can list a directory
on a CIFS mount served by an attacker-controlled server (getdents64()
on the mounted directory):

  BUG: KASAN: slab-out-of-bounds in memchr+0x71/0x80
  Read of size 1 at addr ffff88800e0640cc by task poc/115
  Call Trace:
   dump_stack_lvl+0x64/0x80
   print_report+0xce/0x620
   kasan_report+0xec/0x120
   memchr+0x71/0x80
   filldir64+0x4c/0x6a0
   cifs_filldir.constprop.0+0x9bb/0x1e00
   cifs_readdir+0x2101/0x3380
   iterate_dir+0x19c/0x520
   __x64_sys_getdents64+0x126/0x210
   do_syscall_64+0x107/0x5a0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

Pass the end-of-response pointer down to cifs_filldir() and reject
entries whose name would extend past that boundary.

This bug was discovered by Artiphishell's vTriage pipeline, which
generated a userspace reproducer (an emulated hostile SMB1 server plus
a getdents64() client) that reliably triggers the KASAN report on an
unpatched kernel. The fix below was drafted with the Claude coding
assistant; a userspace reproducer is available on request.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Jay Vadayath <jay@artiphishell.com>

---
 fs/smb/client/readdir.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -952,7 +952,7 @@
 static int cifs_filldir(char *find_entry, struct file *file,
 			struct dir_context *ctx,
 			char *scratch_buf, unsigned int max_len,
-			struct cached_fid *cfid)
+			char *end_of_smb, struct cached_fid *cfid)
 {
 	struct cifsFileInfo *file_info = file->private_data;
 	struct super_block *sb = file_inode(file)->i_sb;
@@ -973,6 +973,11 @@
 			 de.namelen);
 		return -EINVAL;
 	}
+
+	if (de.name + de.namelen > end_of_smb) {
+		cifs_dbg(VFS, "search entry name extends past end of SMB\n");
+		return -EINVAL;
+	}

 	/* skip . and .. since we added them first */
 	if (cifs_entry_is_dot(&de, file_info->srch_inf.unicode))
@@ -1194,7 +1194,7 @@
 		 */
 		*tmp_buf = 0;
 		rc = cifs_filldir(current_entry, file, ctx,
-				  tmp_buf, max_len, cfid);
+				  tmp_buf, max_len, end_of_smb, cfid);
 		if (rc) {
 			if (rc > 0)
 				rc = 0;

Re: [PATCH] smb: client: bound dirent name against end of SMB response in cifs_filldir

[Steve French]

merged into cifs-2.6.git for-next pending additional review and testing

On Fri, Jul 17, 2026 at 7:21 PM Jay Vadayath <jay@artiphishell.com> wrote:
>
> cifs_filldir() copies the entry name out of an SMB1 TRANS2_FIND_FIRST /
> FIND_NEXT response using a length (de.namelen) supplied by the server.
> The kmalloc'd SMB response buffer is bounded, but nothing checks that
> de.name + de.namelen still lies inside that buffer before the eventual
> filldir64() -> verify_dirent_name() -> memchr() reads namelen bytes.
>
> A hostile SMB1 server that returns an oversized FileNameLength in a
> directory entry therefore causes memchr() to read past the end of the
> response slab buffer. Reachable from any user who can list a directory
> on a CIFS mount served by an attacker-controlled server (getdents64()
> on the mounted directory):
>
>   BUG: KASAN: slab-out-of-bounds in memchr+0x71/0x80
>   Read of size 1 at addr ffff88800e0640cc by task poc/115
>   Call Trace:
>    dump_stack_lvl+0x64/0x80
>    print_report+0xce/0x620
>    kasan_report+0xec/0x120
>    memchr+0x71/0x80
>    filldir64+0x4c/0x6a0
>    cifs_filldir.constprop.0+0x9bb/0x1e00
>    cifs_readdir+0x2101/0x3380
>    iterate_dir+0x19c/0x520
>    __x64_sys_getdents64+0x126/0x210
>    do_syscall_64+0x107/0x5a0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Pass the end-of-response pointer down to cifs_filldir() and reject
> entries whose name would extend past that boundary.
>
> This bug was discovered by Artiphishell's vTriage pipeline, which
> generated a userspace reproducer (an emulated hostile SMB1 server plus
> a getdents64() client) that reliably triggers the KASAN report on an
> unpatched kernel. The fix below was drafted with the Claude coding
> assistant; a userspace reproducer is available on request.
>
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Jay Vadayath <jay@artiphishell.com>
>
> ---
>  fs/smb/client/readdir.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> --- a/fs/smb/client/readdir.c
> +++ b/fs/smb/client/readdir.c
> @@ -952,7 +952,7 @@
>  static int cifs_filldir(char *find_entry, struct file *file,
>                         struct dir_context *ctx,
>                         char *scratch_buf, unsigned int max_len,
> -                       struct cached_fid *cfid)
> +                       char *end_of_smb, struct cached_fid *cfid)
>  {
>         struct cifsFileInfo *file_info = file->private_data;
>         struct super_block *sb = file_inode(file)->i_sb;
> @@ -973,6 +973,11 @@
>                          de.namelen);
>                 return -EINVAL;
>         }
> +
> +       if (de.name + de.namelen > end_of_smb) {
> +               cifs_dbg(VFS, "search entry name extends past end of SMB\n");
> +               return -EINVAL;
> +       }
>
>         /* skip . and .. since we added them first */
>         if (cifs_entry_is_dot(&de, file_info->srch_inf.unicode))
> @@ -1194,7 +1194,7 @@
>                  */
>                 *tmp_buf = 0;
>                 rc = cifs_filldir(current_entry, file, ctx,
> -                                 tmp_buf, max_len, cfid);
> +                                 tmp_buf, max_len, end_of_smb, cfid);
>                 if (rc) {
>                         if (rc > 0)
>                                 rc = 0;
>


-- 
Thanks,

Steve