[PATCH] Input: walkera0701 - Fix possible NULL pointer dereference in walkera0701_detach

[PATCH] Input: walkera0701 - Fix possible NULL pointer dereference in walkera0701_detach

[Yue Haibing]

From: YueHaibing <yuehaibing@huawei.com>

KASAN report this:

walkera0701: failed to allocate input device
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN PTI
CPU: 1 PID: 5324 Comm: syz-executor.0 Tainted: G         C        5.1.0-rc3+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
RIP: 0010:input_unregister_device+0x21/0xe0 drivers/input/input.c:2192
Code: 2e 0f 1f 84 00 00 00 00 00 53 48 89 fb e8 07 41 f6 fe 48 8d bb 20 07 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 84 c0 0f 8e 92 00 00 00 80 bb 20 07 00 00
RSP: 0018:ffff8881f58dfd30 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff82460ca9
RDX: 00000000000000e4 RSI: ffffc900013d3000 RDI: 0000000000000720
RBP: 0000000000000000 R08: ffffed103d30caf7 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: dffffc0000000000
R13: ffffffffc1633000 R14: ffffffffc086b320 R15: 1ffff1103eb1bfaf
FS:  00007fa407200700(0000) GS:ffff8881f7300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b33924000 CR3: 00000001e270c006 CR4: 00000000007606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
 walkera0701_detach+0x8e/0xba [walkera0701]
 port_detach+0x73/0x90 [parport]
 bus_for_each_dev+0x154/0x1e0 drivers/base/bus.c:304
 parport_unregister_driver+0x1f8/0x270 [parport]
 __do_sys_delete_module kernel/module.c:1018 [inline]
 __se_sys_delete_module kernel/module.c:961 [inline]
 __x64_sys_delete_module+0x30c/0x480 kernel/module.c:961
 do_syscall_64+0x9f/0x450 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x462e99
Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 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 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fa4071ffc58 EFLAGS: 00000246 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 000000000073bf00 RCX: 0000000000462e99
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000200001c0
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa4072006bc
R13: 00000000004bcca9 R14: 00000000006f6b48 R15: 00000000ffffffff
Modules linked in: walkera0701(-) tps65090_regulator intel_th mptbase adm1031 snd_soc_wm8753 snd_soc_core snd_pcm_dmaengine snd_pcm ac97_bus snd_compress rtc_ds1286 snd_seq_dummy snd_seq snd_timer snd_seq_device snd soundcore comedi(C) i2c_mux_ltc4306 i2c_mux max14577_regulator max14577 usbcore hid cmac mc13783_regulator mc13xxx_regulator_core mc13xxx_core of_mdio fixed_phy libphy iptable_security iptable_raw iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bpfilter ip6_vti ip_vti ip_gre ipip sit tunnel4 ip_tunnel hsr veth netdevsim vxcan batman_adv cfg80211 rfkill chnl_net caif nlmon dummy team bonding vcan bridge stp llc ip6_gre gre ip6_tunnel tunnel6 tun joydev mousedev ppdev tpm kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel ide_pci_generic aesni_intel aes_x86_64 piix crypto_simd cryptd input_leds ide_core psmouse glue_helper intel_agp serio_raw intel_gtt ata_generic agpgart i2c_piix4
 pata_acpi parport_pc parport floppy rtc_cmos sch_fq_codel ip_tables x_tables sha1_ssse3 sha1_generic ipv6 [last unloaded: walkera0701]
Dumping ftrace buffer:
   (ftrace buffer empty)
---[ end trace 17f6dd401f34af3e ]---

In walkera0701_attach(), if input_allocate_device failed,
w->input_dev is set to NULL. But in walkera0701_detach it
is not checked while passing to input_unregister_device(),
this will trigger a NULL pointer dereference issue.

There is also another possible use-after-free issue, when
input_register_device() fails, input_free_device be
called to free input dev, then in walkera0701_detach()
calling input_unregister_device will trigger use-after-free
while accessing input dev

This patch set w->parport to NULL on walkera0701_attach failed,
and only do detach in case attach success.

Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 221bcb24c653 ("Input: walkera0701 - use parallel port device model")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/input/joystick/walkera0701.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
index dce313d..852b8c5 100644
--- a/drivers/input/joystick/walkera0701.c
+++ b/drivers/input/joystick/walkera0701.c
@@ -207,13 +207,13 @@ static void walkera0701_attach(struct parport *pp)
 
 	if (pp->number != walkera0701_pp_no) {
 		pr_debug("Not using parport%d.\n", pp->number);
-		return;
+		goto err_out;
 	}
 
 	if (pp->irq == -1) {
 		pr_err("parport %d does not have interrupt assigned\n",
 			pp->number);
-		return;
+		goto err_out;
 	}
 
 	w->parport = pp;
@@ -228,7 +228,7 @@ static void walkera0701_attach(struct parport *pp)
 
 	if (!w->pardevice) {
 		pr_err("failed to register parport device\n");
-		return;
+		goto err_out;
 	}
 
 	if (parport_negotiate(w->pardevice->port, IEEE1284_MODE_COMPAT)) {
@@ -279,13 +279,15 @@ static void walkera0701_attach(struct parport *pp)
 	input_free_device(w->input_dev);
 err_unregister_device:
 	parport_unregister_device(w->pardevice);
+err_out:
+	w->parport = NULL;
 }
 
 static void walkera0701_detach(struct parport *port)
 {
 	struct walkera_dev *w = &w_dev;
 
-	if (!w->pardevice || w->parport->number != port->number)
+	if (!w->parport)
 		return;
 
 	input_unregister_device(w->input_dev);
-- 
2.7.0

Re: [PATCH] Input: walkera0701 - Fix possible NULL pointer dereference in walkera0701_detach

[YueHaibing]

ping...

On 2019/4/23 22:56, Yue Haibing wrote:
> From: YueHaibing <yuehaibing@huawei.com>
> 
> KASAN report this:
> 
> walkera0701: failed to allocate input device
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] SMP KASAN PTI
> CPU: 1 PID: 5324 Comm: syz-executor.0 Tainted: G         C        5.1.0-rc3+ #8
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
> RIP: 0010:input_unregister_device+0x21/0xe0 drivers/input/input.c:2192
> Code: 2e 0f 1f 84 00 00 00 00 00 53 48 89 fb e8 07 41 f6 fe 48 8d bb 20 07 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 84 c0 0f 8e 92 00 00 00 80 bb 20 07 00 00
> RSP: 0018:ffff8881f58dfd30 EFLAGS: 00010206
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff82460ca9
> RDX: 00000000000000e4 RSI: ffffc900013d3000 RDI: 0000000000000720
> RBP: 0000000000000000 R08: ffffed103d30caf7 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: dffffc0000000000
> R13: ffffffffc1633000 R14: ffffffffc086b320 R15: 1ffff1103eb1bfaf
> FS:  00007fa407200700(0000) GS:ffff8881f7300000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000001b33924000 CR3: 00000001e270c006 CR4: 00000000007606e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Call Trace:
>  walkera0701_detach+0x8e/0xba [walkera0701]
>  port_detach+0x73/0x90 [parport]
>  bus_for_each_dev+0x154/0x1e0 drivers/base/bus.c:304
>  parport_unregister_driver+0x1f8/0x270 [parport]
>  __do_sys_delete_module kernel/module.c:1018 [inline]
>  __se_sys_delete_module kernel/module.c:961 [inline]
>  __x64_sys_delete_module+0x30c/0x480 kernel/module.c:961
>  do_syscall_64+0x9f/0x450 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x462e99
> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 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 c7 c1 bc ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007fa4071ffc58 EFLAGS: 00000246 ORIG_RAX: 00000000000000b0
> RAX: ffffffffffffffda RBX: 000000000073bf00 RCX: 0000000000462e99
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000200001c0
> RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa4072006bc
> R13: 00000000004bcca9 R14: 00000000006f6b48 R15: 00000000ffffffff
> Modules linked in: walkera0701(-) tps65090_regulator intel_th mptbase adm1031 snd_soc_wm8753 snd_soc_core snd_pcm_dmaengine snd_pcm ac97_bus snd_compress rtc_ds1286 snd_seq_dummy snd_seq snd_timer snd_seq_device snd soundcore comedi(C) i2c_mux_ltc4306 i2c_mux max14577_regulator max14577 usbcore hid cmac mc13783_regulator mc13xxx_regulator_core mc13xxx_core of_mdio fixed_phy libphy iptable_security iptable_raw iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bpfilter ip6_vti ip_vti ip_gre ipip sit tunnel4 ip_tunnel hsr veth netdevsim vxcan batman_adv cfg80211 rfkill chnl_net caif nlmon dummy team bonding vcan bridge stp llc ip6_gre gre ip6_tunnel tunnel6 tun joydev mousedev ppdev tpm kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel ide_pci_generic aesni_intel aes_x86_64 piix crypto_simd cryptd input_leds ide_core psmouse glue_helper intel_agp serio_raw intel_gtt ata_generic agpgart i2c_piix4
>  pata_acpi parport_pc parport floppy rtc_cmos sch_fq_codel ip_tables x_tables sha1_ssse3 sha1_generic ipv6 [last unloaded: walkera0701]
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> ---[ end trace 17f6dd401f34af3e ]---
> 
> In walkera0701_attach(), if input_allocate_device failed,
> w->input_dev is set to NULL. But in walkera0701_detach it
> is not checked while passing to input_unregister_device(),
> this will trigger a NULL pointer dereference issue.
> 
> There is also another possible use-after-free issue, when
> input_register_device() fails, input_free_device be
> called to free input dev, then in walkera0701_detach()
> calling input_unregister_device will trigger use-after-free
> while accessing input dev
> 
> This patch set w->parport to NULL on walkera0701_attach failed,
> and only do detach in case attach success.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: 221bcb24c653 ("Input: walkera0701 - use parallel port device model")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/input/joystick/walkera0701.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
> index dce313d..852b8c5 100644
> --- a/drivers/input/joystick/walkera0701.c
> +++ b/drivers/input/joystick/walkera0701.c
> @@ -207,13 +207,13 @@ static void walkera0701_attach(struct parport *pp)
>  
>  	if (pp->number != walkera0701_pp_no) {
>  		pr_debug("Not using parport%d.\n", pp->number);
> -		return;
> +		goto err_out;
>  	}
>  
>  	if (pp->irq == -1) {
>  		pr_err("parport %d does not have interrupt assigned\n",
>  			pp->number);
> -		return;
> +		goto err_out;
>  	}
>  
>  	w->parport = pp;
> @@ -228,7 +228,7 @@ static void walkera0701_attach(struct parport *pp)
>  
>  	if (!w->pardevice) {
>  		pr_err("failed to register parport device\n");
> -		return;
> +		goto err_out;
>  	}
>  
>  	if (parport_negotiate(w->pardevice->port, IEEE1284_MODE_COMPAT)) {
> @@ -279,13 +279,15 @@ static void walkera0701_attach(struct parport *pp)
>  	input_free_device(w->input_dev);
>  err_unregister_device:
>  	parport_unregister_device(w->pardevice);
> +err_out:
> +	w->parport = NULL;
>  }
>  
>  static void walkera0701_detach(struct parport *port)
>  {
>  	struct walkera_dev *w = &w_dev;
>  
> -	if (!w->pardevice || w->parport->number != port->number)
> +	if (!w->parport)
>  		return;
>  
>  	input_unregister_device(w->input_dev);
> 

Re: [PATCH] Input: walkera0701 - Fix possible NULL pointer dereference in walkera0701_detach

[Sudip Mukherjee]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

On Tue, Apr 23, 2019 at 10:56:37PM +0800, Yue Haibing wrote:
> From: YueHaibing <yuehaibing@huawei.com>
> 
> KASAN report this:

<snip>

>  
>  static void walkera0701_detach(struct parport *port)
>  {
>  	struct walkera_dev *w = &w_dev;
>  
> -	if (!w->pardevice || w->parport->number != port->number)
> +	if (!w->parport)

It doesn't look correct. This way the detach function will never know the
port number to which it is attached, and as a result it will try to do
detach() with all the ports in the system.
Please check the attached patch and maybe (if possible) ask Hulk Robot
to verify if it fixes the problem.

--
Regards
Sudip

[-- Attachment #2: 0001-Input-walkera0701-Fix-possible-NULL-pointer-derefere.patch --]
[-- Type: text/x-diff, Size: 1406 bytes --]

From 0338a89a873e7df57707852402f90bb0d6626f12 Mon Sep 17 00:00:00 2001
From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Date: Wed, 16 Oct 2019 16:08:43 +0100
Subject: [PATCH] Input: walkera0701 - Fix possible NULL pointer dereference

If walkera0701_attach() fails and input_dev is made NULL then we are
unregistering the pardevice but it still has the pointer to the dev
which has now been released. And as a result in the walkera0701_detach()
it will now try to do input_unregister_device() with a NULL pointer.
We should mark the pardevice as NULL when it is unregistered.

Reported-by: Hulk Robot <hulkci@huawei.com>
Reported-by: Yue Haibing <yuehaibing@huawei.com>
Fixes: 221bcb24c653 ("Input: walkera0701 - use parallel port device model")
Cc: stable@vger.kernel.org # v4.4+
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 drivers/input/joystick/walkera0701.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
index 56abc8c6c763..d8ae1329bf00 100644
--- a/drivers/input/joystick/walkera0701.c
+++ b/drivers/input/joystick/walkera0701.c
@@ -275,6 +275,7 @@ static void walkera0701_attach(struct parport *pp)
 	input_free_device(w->input_dev);
 err_unregister_device:
 	parport_unregister_device(w->pardevice);
+	w->pardevice = NULL;
 }
 
 static void walkera0701_detach(struct parport *port)
-- 
2.11.0