All of lore.kernel.org
 help / color / mirror / Atom feed
From: Haimin Zhang <tcs.kernel@gmail.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>, linux-fsdevel@vger.kernel.org
Cc: Haimin Zhang <tcs_kernel@tencent.com>, TCS Robot <tcs_robot@tencent.com>
Subject: [PATCH] fhandle: add __GFP_ZERO flag for kmalloc in function do_sys_name_to_handle
Date: Tue, 22 Mar 2022 10:13:26 +0800	[thread overview]
Message-ID: <20220322021326.20162-1-tcs.kernel@gmail.com> (raw)

From: Haimin Zhang <tcs_kernel@tencent.com>

Add __GFP_ZERO flag for kmalloc in function do_sys_name_to_handle to
initialize the buffer of a file_handle variable.

Reported-by: TCS Robot <tcs_robot@tencent.com>
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
---
This can cause a two-bytes-size kernel-info-leak problem.
1. do_sys_name_to_handle calls kmalloc to allocate the buffer of a file_handle variable, but doesn't zero it properly.
```
kmalloc build/../include/linux/slab.h:586 [inline]
 do_sys_name_to_handle build/../fs/fhandle.c:40 [inline]
 __do_sys_name_to_handle_at build/../fs/fhandle.c:109 [inline]
 __se_sys_name_to_handle_at+0x470/0x990 build/../fs/fhandle.c:93
 __x64_sys_name_to_handle_at+0x15d/0x1b0 build/../fs/fhandle.c:93
 do_syscall_x64 build/../arch/x86/entry/common.c:51 [inline]
 do_syscall_64+0x54/0xd0 build/../arch/x86/entry/common.c:82
 entry_SYSCALL_64_after_hwframe+0x44/0xae
```
2. do_sys_name_to_handle calls exportfs_encode_fh to fill the content of the variable.
3. If the inode is a fat node, exportfs_encode_fh will call fat_encode_fh_nostale to further process.
```
#0  fat_encode_fh_nostale (inode=inode@entry=0xffff88815d9286a8, fh=fh@entry=0xffff88815ce7f008, lenp=lenp@entry=0xffff88814f5c3e4c,
    parent=parent@entry=0x0 <fixed_percpu_data>) at ../fs/fat/nfs.c:102
#1  0xffffffff8375cbe1 in exportfs_encode_inode_fh (inode=0xffff88815d9286a8, fid=0xffff88815ce7f008, max_len=0xffff88814f5c3e4c,
    parent=0x0 <fixed_percpu_data>) at ../fs/exportfs/expfs.c:391
#2  exportfs_encode_fh (dentry=0x0 <fixed_percpu_data>, dentry@entry=0xffff8881255cf480, fid=fid@entry=0xffff88815ce7f008,
    max_len=max_len@entry=0xffff88814f5c3e4c, connectable=connectable@entry=0x0) at ../fs/exportfs/expfs.c:413
#3  0xffffffff82a88c9b in do_sys_name_to_handle (path=0xffff88814f5c3e38, ufh=0x20000500, mnt_id=0x20000540) at ../fs/fhandle.c:49
#4  __do_sys_name_to_handle_at (dfd=<optimized out>, name=0x0 <fixed_percpu_data>, flag=<optimized out>, handle=<optimized out>, mnt_id=<optimized out>)
    at ../fs/fhandle.c:109
#5  __se_sys_name_to_handle_at (dfd=0xffff88815ce7f000, dfd@entry=0xffffff9c, name=0x0, name@entry=0x200004c0, handle=handle@entry=0x20000500,
    mnt_id=mnt_id@entry=0x20000540, flag=flag@entry=0x1000) at ../fs/fhandle.c:93
#6  0xffffffff82a8870d in __x64_sys_name_to_handle_at (regs=<optimized out>) at ../fs/fhandle.c:93
#7  0xffffffff8fb1c264 in do_syscall_x64 (regs=0xffff88814f5c3f58, nr=0x12f) at ../arch/x86/entry/common.c:51
#8  do_syscall_64 (regs=0xffff88814f5c3f58, nr=0x12f) at ../arch/x86/entry/common.c:82
#9  0xffffffff8fc00068 in entry_SYSCALL_64 () at ../net/unix/af_unix.c:3364
#10 0x0000000000000000 in ?? ()
```
4. If the argument parent is NULL, the lenp will be 3, it means that fat_encode_fh_nostale modifies 12 bytes of the buffer. But actually it just modifies 10 bytes.
```
struct fat_fid {
	u32 i_gen;
	u32 i_pos_low;
	u16 i_pos_hi;
	// The following fields will be assigned on if parent isn't NULL, 
	u16 parent_i_pos_hi;
	u32 parent_i_pos_low;
	u32 parent_i_gen;
};
```
5. When it returns to do_sys_name_to_handle, the whole 12 bytes will be copied to user space.
```
 copy_to_user build/../include/linux/uaccess.h:209 [inline]
 do_sys_name_to_handle build/../fs/fhandle.c:73 [inline]
 __do_sys_name_to_handle_at build/../fs/fhandle.c:109 [inline]
 __se_sys_name_to_handle_at+0x86b/0x990 build/../fs/fhandle.c:93
 __x64_sys_name_to_handle_at+0x15d/0x1b0 build/../fs/fhandle.c:93
 do_syscall_x64 build/../arch/x86/entry/common.c:51 [inline]
 do_syscall_64+0x54/0xd0 build/../arch/x86/entry/common.c:82
 entry_SYSCALL_64_after_hwframe+0x44/0xae
```
6. The following is debug information:
Bytes 18-19 of 20 are uninitialized
Memory access of size 20 starts at ffff88815e77a1e0
Data copied to user address 0000000020000500

 fs/fhandle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fhandle.c b/fs/fhandle.c
index 6630c69c23a2..be6b9ed12dfd 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -38,7 +38,7 @@ static long do_sys_name_to_handle(struct path *path,
 		return -EINVAL;
 
 	handle = kmalloc(sizeof(struct file_handle) + f_handle.handle_bytes,
-			 GFP_KERNEL);
+			 GFP_KERNEL | __GFP_ZERO);
 	if (!handle)
 		return -ENOMEM;
 
-- 
2.32.0 (Apple Git-132)


             reply	other threads:[~2022-03-22  2:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-22  2:13 Haimin Zhang [this message]
2022-03-22  2:41 ` [PATCH] fhandle: add __GFP_ZERO flag for kmalloc in function do_sys_name_to_handle Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220322021326.20162-1-tcs.kernel@gmail.com \
    --to=tcs.kernel@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tcs_kernel@tencent.com \
    --cc=tcs_robot@tencent.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.