diff mbox series

[RFC,12/20] input: remove f_version abuse

Message ID 20240830-vfs-file-f_version-v1-12-6d3e4816aa7b@kernel.org (mailing list archive)
State New
Headers show
Series file: remove f_version | expand

Commit Message

Christian Brauner Aug. 30, 2024, 1:04 p.m. UTC 
Remove the f_version abuse from input. Use seq_private_open() to stash
the information for poll.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 drivers/input/input.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Jan Kara Sept. 3, 2024, 11:40 a.m. UTC | #1 
On Fri 30-08-24 15:04:53, Christian Brauner wrote:
> Remove the f_version abuse from input. Use seq_private_open() to stash
> the information for poll.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  drivers/input/input.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 54c57b267b25..b03ae43707d8 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1081,9 +1081,11 @@ static inline void input_wakeup_procfs_readers(void)
>  
>  static __poll_t input_proc_devices_poll(struct file *file, poll_table *wait)
>  {
> +	struct seq_file *m = file->private_data;
> +
>  	poll_wait(file, &input_devices_poll_wait, wait);
> -	if (file->f_version != input_devices_state) {
> -		file->f_version = input_devices_state;
> +	if (*(u64 *)m->private != input_devices_state) {
> +		*(u64 *)m->private = input_devices_state;
>  		return EPOLLIN | EPOLLRDNORM;
>  	}
>  
> @@ -1210,7 +1212,7 @@ static const struct seq_operations input_devices_seq_ops = {
>  
>  static int input_proc_devices_open(struct inode *inode, struct file *file)
>  {
> -	return seq_open(file, &input_devices_seq_ops);
> +	return seq_open_private(file, &input_devices_seq_ops, sizeof(u64));
>  }
>  
>  static const struct proc_ops input_devices_proc_ops = {
> 
> -- 
> 2.45.2
>
Lai, Yi Sept. 12, 2024, 2:52 a.m. UTC | #2 
Hi Christian Brauner,

Greetings!

I used Syzkaller and found that there is BUG: unable to handle kernel paging request in input_proc_devices_poll in next-20240909.

After bisection and the first bad commit is:
"
7c3d158418c2 input: remove f_version abuse
"

All detailed into can be found at:
https://github.com/laifryiee/syzkaller_logs/tree/main/240911_155303_input_proc_devices_poll
Syzkaller repro code:
https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/repro.c
Syzkaller repro syscall steps:
https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/repro.prog
Syzkaller report:
https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/repro.report
Kconfig(make olddefconfig):
https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/kconfig_origin
Bisect info:
https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/bisect_info.log
bzImage:
https://github.com/laifryiee/syzkaller_logs/raw/main/240911_155303_input_proc_devices_poll/bzImage_100cc857359b5d731407d1038f7e76cd0e871d94
Issue dmesg:
https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/100cc857359b5d731407d1038f7e76cd0e871d94_dmesg.log

"
[   23.266063] ==================================================================
[   23.268350] BUG: KASAN: slab-out-of-bounds in input_proc_devices_poll+0x113/0x140
[   23.270806] Read of size 8 at addr ffff88801101fa40 by task repro/729
[   23.272537] 
[   23.272980] CPU: 1 UID: 0 PID: 729 Comm: repro Not tainted 6.11.0-rc7-next-20240909-100cc857359b-dirty #1
[   23.274230] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[   23.275200] Call Trace:
[   23.275432]  <TASK>
[   23.275633]  dump_stack_lvl+0xea/0x150
[   23.275972]  print_report+0xce/0x610
[   23.276269]  ? input_proc_devices_poll+0x113/0x140
[   23.276650]  ? kasan_complete_mode_report_info+0x40/0x200
[   23.277058]  ? input_proc_devices_poll+0x113/0x140
[   23.277400]  kasan_report+0xcc/0x110
[   23.277668]  ? input_proc_devices_poll+0x113/0x140
[   23.277989]  ? __pfx___pollwait+0x10/0x10
[   23.278288]  __asan_report_load8_noabort+0x18/0x20
[   23.278634]  input_proc_devices_poll+0x113/0x140
[   23.278963]  ? __pfx_input_proc_devices_poll+0x10/0x10
[   23.279325]  proc_reg_poll+0x210/0x2e0
[   23.279607]  ? __pfx_proc_reg_poll+0x10/0x10
[   23.279917]  do_sys_poll+0x521/0xdd0
[   23.280188]  ? __pfx_do_sys_poll+0x10/0x10
[   23.280485]  ? __kasan_check_read+0x15/0x20
[   23.280791]  ? mark_lock.part.0+0xf3/0x17b0
[   23.281101]  ? __pfx_mark_lock.part.0+0x10/0x10
[   23.281427]  ? __kasan_check_read+0x15/0x20
[   23.281736]  ? mark_lock.part.0+0xf3/0x17b0
[   23.282039]  ? mutex_unlock+0x16/0x20
[   23.282311]  ? seq_read_iter+0x72/0x1300
[   23.282604]  ? __pfx_mark_lock.part.0+0x10/0x10
[   23.282935]  ? __pfx___pollwait+0x10/0x10
[   23.283235]  ? __pfx_pollwake+0x10/0x10
[   23.283526]  ? __pfx___lock_acquire+0x10/0x10
[   23.283848]  ? __this_cpu_preempt_check+0x21/0x30
[   23.284200]  ? __this_cpu_preempt_check+0x21/0x30
[   23.284542]  ? lock_release+0x441/0x870
[   23.284825]  ? __sanitizer_cov_trace_cmp8+0x1c/0x30
[   23.285180]  ? timespec64_add_safe+0x192/0x220
[   23.285505]  ? __pfx_timespec64_add_safe+0x10/0x10
[   23.285851]  ? __sanitizer_cov_trace_const_cmp1+0x1e/0x30
[   23.286236]  ? __sanitizer_cov_trace_const_cmp8+0x1c/0x30
[   23.286622]  ? ktime_get_ts64+0x1db/0x2e0
[   23.286925]  __x64_sys_poll+0x1bf/0x560
[   23.287205]  ? __pfx___x64_sys_poll+0x10/0x10
[   23.287524]  x64_sys_call+0x1294/0x2140
[   23.287808]  do_syscall_64+0x6d/0x140
[   23.288083]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[   23.288457] RIP: 0033:0x7faf33c3ee5d
[   23.288721] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 93 af 1b 00 f7 d8 64 89 01 48
[   23.289988] RSP: 002b:00007ffff3de1fe8 EFLAGS: 00000207 ORIG_RAX: 0000000000000007
[   23.290531] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007faf33c3ee5d
[   23.291025] RDX: 0000000000000029 RSI: 0000000000000005 RDI: 0000000020000040
[   23.291519] RBP: 00007ffff3de2000 R08: 00007ffff3de2000 R09: 00007ffff3de2000
[   23.292201] R10: 00007ffff3de2000 R11: 0000000000000207 R12: 00007ffff3de2158
[   23.292800] R13: 0000000000401810 R14: 0000000000403e08 R15: 00007faf33e55000
[   23.293320]  </TASK>
[   23.293487] 
[   23.293609] Allocated by task 1:
[   23.293862]  kasan_save_stack+0x2c/0x60
[   23.294153]  kasan_save_track+0x18/0x40
[   23.294435]  kasan_save_alloc_info+0x3c/0x50
[   23.294744]  __kasan_kmalloc+0x88/0xa0
[   23.295019]  __kmalloc_noprof+0x1cd/0x4a0
[   23.295316]  cgroup_mkdir+0x282/0x1320
[   23.295602]  kernfs_iop_mkdir+0x15a/0x1f0
[   23.295899]  vfs_mkdir+0x57d/0x860
[   23.296157]  do_mkdirat+0x2e2/0x3b0
[   23.296414]  __x64_sys_mkdir+0xfd/0x150
[   23.296692]  x64_sys_call+0x1c5a/0x2140
[   23.296974]  do_syscall_64+0x6d/0x140
[   23.297246]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[   23.297611] 
[   23.297735] The buggy address belongs to the object at ffff88801101e000
[   23.297735]  which belongs to the cache kmalloc-4k of size 4096
[   23.298583] The buggy address is located 4584 bytes to the right of
[   23.298583]  allocated 2136-byte region [ffff88801101e000, ffff88801101e858)
[   23.299489] 
[   23.299611] The buggy address belongs to the physical page:
[   23.300004] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff88801101c000 pfn:0x11018
[   23.300643] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[   23.301177] flags: 0xfffffc0000240(workingset|head|node=0|zone=1|lastcpupid=0x1fffff)
[   23.301729] page_type: f5(slab)
[   23.301968] raw: 000fffffc0000240 ffff88800d442140 ffffea00004e3610 ffffea0000446410
[   23.302505] raw: ffff88801101c000 0000000000040002 00000001f5000000 0000000000000000
[   23.303048] head: 000fffffc0000240 ffff88800d442140 ffffea00004e3610 ffffea0000446410
[   23.303597] head: ffff88801101c000 0000000000040002 00000001f5000000 0000000000000000
[   23.304147] head: 000fffffc0000003 ffffea0000440601 ffffffffffffffff 0000000000000000
[   23.304692] head: ffff888000000008 0000000000000000 00000000ffffffff 0000000000000000
[   23.305238] page dumped because: kasan: bad access detected
[   23.305632] 
[   23.305752] Memory state around the buggy address:
[   23.306096]  ffff88801101f900: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   23.306600]  ffff88801101f980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   23.307102] >ffff88801101fa00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   23.307612]                                            ^
[   23.307984]  ffff88801101fa80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   23.308490]  ffff88801101fb00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   23.308993] ==================================================================
[   23.309546] Disabling lock debugging due to kernel taint

"

I hope you find it useful.

Regards,
Yi Lai

---

If you don't need the following environment to reproduce the problem or if you
already have one reproduced environment, please ignore the following information.

How to reproduce:
git clone https://gitlab.com/xupengfe/repro_vm_env.git
cd repro_vm_env
tar -xvf repro_vm_env.tar.gz
cd repro_vm_env; ./start3.sh  // it needs qemu-system-x86_64 and I used v7.1.0
  // start3.sh will load bzImage_2241ab53cbb5cdb08a6b2d4688feb13971058f65 v6.2-rc5 kernel
  // You could change the bzImage_xxx as you want
  // Maybe you need to remove line "-drive if=pflash,format=raw,readonly=on,file=./OVMF_CODE.fd \" for different qemu version
You could use below command to log in, there is no password for root.
ssh -p 10023 root@localhost

After login vm(virtual machine) successfully, you could transfer reproduced
binary to the vm by below way, and reproduce the problem in vm:
gcc -pthread -o repro repro.c
scp -P 10023 repro root@localhost:/root/

Get the bzImage for target kernel:
Please use target kconfig and copy it to kernel_src/.config
make olddefconfig
make -jx bzImage           //x should equal or less than cpu num your pc has

Fill the bzImage file into above start3.sh to load the target kernel in vm.

Tips:
If you already have qemu-system-x86_64, please ignore below info.
If you want to install qemu v7.1.0 version:
git clone https://github.com/qemu/qemu.git
cd qemu
git checkout -f v7.1.0
mkdir build
cd build
yum install -y ninja-build.x86_64
yum -y install libslirp-devel.x86_64
../configure --target-list=x86_64-softmmu --enable-kvm --enable-vnc --enable-gtk --enable-sdl --enable-usb-redir --enable-slirp
make
make install 

On Fri, Aug 30, 2024 at 03:04:53PM +0200, Christian Brauner wrote:
> Remove the f_version abuse from input. Use seq_private_open() to stash
> the information for poll.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  drivers/input/input.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 54c57b267b25..b03ae43707d8 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1081,9 +1081,11 @@ static inline void input_wakeup_procfs_readers(void)
>  
>  static __poll_t input_proc_devices_poll(struct file *file, poll_table *wait)
>  {
> +	struct seq_file *m = file->private_data;
> +
>  	poll_wait(file, &input_devices_poll_wait, wait);
> -	if (file->f_version != input_devices_state) {
> -		file->f_version = input_devices_state;
> +	if (*(u64 *)m->private != input_devices_state) {
> +		*(u64 *)m->private = input_devices_state;
>  		return EPOLLIN | EPOLLRDNORM;
>  	}
>  
> @@ -1210,7 +1212,7 @@ static const struct seq_operations input_devices_seq_ops = {
>  
>  static int input_proc_devices_open(struct inode *inode, struct file *file)
>  {
> -	return seq_open(file, &input_devices_seq_ops);
> +	return seq_open_private(file, &input_devices_seq_ops, sizeof(u64));
>  }
>  
>  static const struct proc_ops input_devices_proc_ops = {
> 
> -- 
> 2.45.2
>
Christian Brauner Sept. 12, 2024, 10:02 a.m. UTC | #3 
On Thu, Sep 12, 2024 at 10:52:16AM GMT, Lai, Yi wrote:
> Hi Christian Brauner,
> 
> Greetings!
> 
> I used Syzkaller and found that there is BUG: unable to handle kernel paging request in input_proc_devices_poll in next-20240909.
> 
> After bisection and the first bad commit is:
> "
> 7c3d158418c2 input: remove f_version abuse
> "
> 
> All detailed into can be found at:
> https://github.com/laifryiee/syzkaller_logs/tree/main/240911_155303_input_proc_devices_poll
> Syzkaller repro code:
> https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/repro.c
> Syzkaller repro syscall steps:
> https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/repro.prog
> Syzkaller report:
> https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/repro.report
> Kconfig(make olddefconfig):
> https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/kconfig_origin
> Bisect info:
> https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/bisect_info.log
> bzImage:
> https://github.com/laifryiee/syzkaller_logs/raw/main/240911_155303_input_proc_devices_poll/bzImage_100cc857359b5d731407d1038f7e76cd0e871d94
> Issue dmesg:
> https://github.com/laifryiee/syzkaller_logs/blob/main/240911_155303_input_proc_devices_poll/100cc857359b5d731407d1038f7e76cd0e871d94_dmesg.log

Thanks for all the info. I see what the issue is and pushed a fix out.
diff mbox series

Patch

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 54c57b267b25..b03ae43707d8 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1081,9 +1081,11 @@  static inline void input_wakeup_procfs_readers(void)
 
 static __poll_t input_proc_devices_poll(struct file *file, poll_table *wait)
 {
+	struct seq_file *m = file->private_data;
+
 	poll_wait(file, &input_devices_poll_wait, wait);
-	if (file->f_version != input_devices_state) {
-		file->f_version = input_devices_state;
+	if (*(u64 *)m->private != input_devices_state) {
+		*(u64 *)m->private = input_devices_state;
 		return EPOLLIN | EPOLLRDNORM;
 	}
 
@@ -1210,7 +1212,7 @@  static const struct seq_operations input_devices_seq_ops = {
 
 static int input_proc_devices_open(struct inode *inode, struct file *file)
 {
-	return seq_open(file, &input_devices_seq_ops);
+	return seq_open_private(file, &input_devices_seq_ops, sizeof(u64));
 }
 
 static const struct proc_ops input_devices_proc_ops = {