2021-10-11 12:07:37

by Yang Yingliang

[permalink] [raw]
Subject: [PATCH 1/2] rtc: class: check return value when calling dev_set_name()

I got a null-ptr-deref report when doing fault injection test:

BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] SMP KASAN PTI
CPU: 2 PID: 925 Comm: 29 Not tainted 5.15.0-rc3-00111-gf5dad42ed4fe-dirty #487 5b4d17fc3275713934c1a9cb26349fbabf82adbf
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:strcmp+0xc/0x20
Code: 17 48 83 c6 01 44 0f b6 46 ff 48 83 c1 01 44 88 41 ff 45 84 c0 75 e5 c3 c6 01 00 c3 66 90 31 c0 eb 08 48 83 c0 01 84 d2 74 0f <0f> b6 14 07 3a 14 06 74 ef 19 c0 83 c8 01 c3 31 c0 c3 66 90 48 85
RSP: 0018:ffffc900025af368 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 1ffff920004b5e6f RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8ebcf680 RDI: 0000000000000000
RBP: ffff888014746000 R08: ffffed102097e3fa R09: ffffed102097e3fa
R10: ffff888104bf1fcb R11: ffffed102097e3f9 R12: ffff888014746040
R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880147468c0
FS: 00007f783e6d5500(0000) GS:ffff888104a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000008cee002 CR4: 0000000000770ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
__devm_rtc_register_device.cold.7+0x16a/0x2df
? rtc_suspend+0x330/0x330
? irqentry_exit+0x32/0x80
? __sanitizer_cov_trace_pc+0x1d/0x50
? irqentry_exit+0x32/0x80
? trace_hardirqs_on+0x63/0x2d0
? rtc_ktime_to_tm+0x120/0x120
? tracer_hardirqs_on+0x36/0x530
? _raw_spin_unlock_irqrestore+0x4b/0x5d
? _raw_spin_unlock_irqrestore+0x54/0x5d
? __sanitizer_cov_trace_pc+0x1d/0x50
? write_comp_data+0x2a/0x90
? __sanitizer_cov_trace_pc+0x1d/0x50
rv3029_probe+0x4b1/0x770 [rtc_rv3029c2]
? rv3029_hwmon_show_update_interval+0x160/0x160 [rtc_rv3029c2]
? write_comp_data+0x2a/0x90
? _raw_spin_unlock_irqrestore+0x4b/0x5d
? tracer_hardirqs_on+0x36/0x530
? rv3029_nvram_write+0x40/0x40 [rtc_rv3029c2]
? rv3029_set_time+0x350/0x350 [rtc_rv3029c2]
? __sanitizer_cov_trace_pc+0x1d/0x50
rv3029_i2c_probe+0x141/0x180 [rtc_rv3029c2]
? rv3029_probe+0x770/0x770 [rtc_rv3029c2]
i2c_device_probe+0xa07/0xbb0
? i2c_device_match+0x110/0x110
really_probe+0x285/0xc30

If dev_set_name() fails, dev_name() is null, it causes null-ptr-deref,
we need check the return value of dev_set_name().

Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Yang Yingliang <[email protected]>
---
drivers/rtc/class.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index f77bc089eb6b..1f18c39a4b82 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -363,7 +363,9 @@ struct rtc_device *devm_rtc_allocate_device(struct device *dev)

rtc->id = id;
rtc->dev.parent = dev;
- dev_set_name(&rtc->dev, "rtc%d", id);
+ err = dev_set_name(&rtc->dev, "rtc%d", id);
+ if (err)
+ return ERR_PTR(err);

err = devm_add_action_or_reset(dev, devm_rtc_release_device, rtc);
if (err)
--
2.25.1


2021-10-11 12:08:09

by Yang Yingliang

[permalink] [raw]
Subject: [PATCH 2/2] rtc: class: return error code when cdev_device_add() failed

I got a null-ptr-deref report when doing fault injection test:

general protection fault, probably for non-canonical address 0xdffffc0000000022: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000110-0x0000000000000117]
CPU: 1 PID: 1028 Comm: 33 Not tainted 5.15.0-rc3-00111-gf5dad42ed4fe-dirty #481 2a70b3e6ca240b8638beac7ef491cce6183bbec7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:device_del+0x132/0xdc0
Code: 48 c1 ea 03 80 3c 02 00 0f 85 4f 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b 7b 48 4d 8d a7 10 01 00 00 4c 89 e2 48 c1 ea 03 <0f> b6 04 02 84 c0 74 06 0f 8e 7f 0a 00 00 45 0f b6 b7 10 01 00 00
RSP: 0018:ffffc90002e876b8 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: ffff88801eb84000 RCX: ffffffff97227644
RDX: 0000000000000022 RSI: ffff8880146a0000 RDI: 0000000000000002
RBP: ffff88801eb84120 R08: fffffbfff349a60d R09: fffffbfff349a60d
R10: ffffc90002e876b8 R11: fffffbfff349a60c R12: 0000000000000110
R13: 0000000000000001 R14: ffffc90002e87848 R15: 0000000000000000
FS: 00007fa514973500(0000) GS:ffff888104600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fa51474ccb0 CR3: 000000002bcb0001 CR4: 0000000000770ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
? write_comp_data+0x2a/0x90
? cleanup_glue_dir+0x260/0x260
? is_rtc_hctosys.isra.0+0xb9/0xf0
? rtc_proc_show+0x440/0x440
? rcu_read_lock_held_common+0xe/0xa0
? rcu_read_lock_sched_held+0x62/0xe0
cdev_device_del+0x1a/0x80
devm_rtc_unregister_device+0x37/0x80
release_nodes+0xc3/0x3b0

If cdev_device_add() fails, 'dev->p' is not set, it causes
null-ptr-deref when calling cdev_device_del(), return error
code when cdev_device_add() failed to fix this.

Reported-by: Hulk Robot <[email protected]>
Fixes: 3068a254d5519 ("rtc: introduce new registration method")
Signed-off-by: Yang Yingliang <[email protected]>
---
drivers/rtc/class.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 1f18c39a4b82..76422faee05b 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -399,12 +399,14 @@ int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc)
rtc_dev_prepare(rtc);

err = cdev_device_add(&rtc->char_dev, &rtc->dev);
- if (err)
+ if (err) {
dev_warn(rtc->dev.parent, "failed to add char device %d:%d\n",
MAJOR(rtc->dev.devt), rtc->id);
- else
+ return err;
+ } else {
dev_dbg(rtc->dev.parent, "char device (%d:%d)\n",
MAJOR(rtc->dev.devt), rtc->id);
+ }

rtc_proc_add_device(rtc);

--
2.25.1

2021-10-11 12:13:40

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtc: class: return error code when cdev_device_add() failed

On 11/10/2021 16:03:02+0800, Yang Yingliang wrote:
> I got a null-ptr-deref report when doing fault injection test:
>
> general protection fault, probably for non-canonical address 0xdffffc0000000022: 0000 [#1] SMP KASAN PTI
> KASAN: null-ptr-deref in range [0x0000000000000110-0x0000000000000117]
> CPU: 1 PID: 1028 Comm: 33 Not tainted 5.15.0-rc3-00111-gf5dad42ed4fe-dirty #481 2a70b3e6ca240b8638beac7ef491cce6183bbec7
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:device_del+0x132/0xdc0
> Code: 48 c1 ea 03 80 3c 02 00 0f 85 4f 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b 7b 48 4d 8d a7 10 01 00 00 4c 89 e2 48 c1 ea 03 <0f> b6 04 02 84 c0 74 06 0f 8e 7f 0a 00 00 45 0f b6 b7 10 01 00 00
> RSP: 0018:ffffc90002e876b8 EFLAGS: 00010206
> RAX: dffffc0000000000 RBX: ffff88801eb84000 RCX: ffffffff97227644
> RDX: 0000000000000022 RSI: ffff8880146a0000 RDI: 0000000000000002
> RBP: ffff88801eb84120 R08: fffffbfff349a60d R09: fffffbfff349a60d
> R10: ffffc90002e876b8 R11: fffffbfff349a60c R12: 0000000000000110
> R13: 0000000000000001 R14: ffffc90002e87848 R15: 0000000000000000
> FS: 00007fa514973500(0000) GS:ffff888104600000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fa51474ccb0 CR3: 000000002bcb0001 CR4: 0000000000770ee0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Call Trace:
> ? write_comp_data+0x2a/0x90
> ? cleanup_glue_dir+0x260/0x260
> ? is_rtc_hctosys.isra.0+0xb9/0xf0
> ? rtc_proc_show+0x440/0x440
> ? rcu_read_lock_held_common+0xe/0xa0
> ? rcu_read_lock_sched_held+0x62/0xe0
> cdev_device_del+0x1a/0x80
> devm_rtc_unregister_device+0x37/0x80
> release_nodes+0xc3/0x3b0
>
> If cdev_device_add() fails, 'dev->p' is not set, it causes
> null-ptr-deref when calling cdev_device_del(), return error
> code when cdev_device_add() failed to fix this.
>
> Reported-by: Hulk Robot <[email protected]>
> Fixes: 3068a254d5519 ("rtc: introduce new registration method")
> Signed-off-by: Yang Yingliang <[email protected]>
> ---
> drivers/rtc/class.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index 1f18c39a4b82..76422faee05b 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -399,12 +399,14 @@ int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc)
> rtc_dev_prepare(rtc);
>
> err = cdev_device_add(&rtc->char_dev, &rtc->dev);
> - if (err)
> + if (err) {
> dev_warn(rtc->dev.parent, "failed to add char device %d:%d\n",
> MAJOR(rtc->dev.devt), rtc->id);
> - else
> + return err;

As the checkpatch warning indicates, registering the character device as
always been optional. d5ed9177f64fe95d9de79e6504d41612d9127e8a is the
commit you want to fix.

> + } else {
> dev_dbg(rtc->dev.parent, "char device (%d:%d)\n",
> MAJOR(rtc->dev.devt), rtc->id);
> + }
>
> rtc_proc_add_device(rtc);
>
> --
> 2.25.1
>

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2021-10-11 13:10:44

by Yang Yingliang

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtc: class: return error code when cdev_device_add() failed


Hi,

On 2021/10/11 16:36, Alexandre Belloni wrote:
> On 11/10/2021 16:03:02+0800, Yang Yingliang wrote:
>> I got a null-ptr-deref report when doing fault injection test:
>>
>> general protection fault, probably for non-canonical address 0xdffffc0000000022: 0000 [#1] SMP KASAN PTI
>> KASAN: null-ptr-deref in range [0x0000000000000110-0x0000000000000117]
>> CPU: 1 PID: 1028 Comm: 33 Not tainted 5.15.0-rc3-00111-gf5dad42ed4fe-dirty #481 2a70b3e6ca240b8638beac7ef491cce6183bbec7
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
>> RIP: 0010:device_del+0x132/0xdc0
>> Code: 48 c1 ea 03 80 3c 02 00 0f 85 4f 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b 7b 48 4d 8d a7 10 01 00 00 4c 89 e2 48 c1 ea 03 <0f> b6 04 02 84 c0 74 06 0f 8e 7f 0a 00 00 45 0f b6 b7 10 01 00 00
>> RSP: 0018:ffffc90002e876b8 EFLAGS: 00010206
>> RAX: dffffc0000000000 RBX: ffff88801eb84000 RCX: ffffffff97227644
>> RDX: 0000000000000022 RSI: ffff8880146a0000 RDI: 0000000000000002
>> RBP: ffff88801eb84120 R08: fffffbfff349a60d R09: fffffbfff349a60d
>> R10: ffffc90002e876b8 R11: fffffbfff349a60c R12: 0000000000000110
>> R13: 0000000000000001 R14: ffffc90002e87848 R15: 0000000000000000
>> FS: 00007fa514973500(0000) GS:ffff888104600000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007fa51474ccb0 CR3: 000000002bcb0001 CR4: 0000000000770ee0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> PKRU: 55555554
>> Call Trace:
>> ? write_comp_data+0x2a/0x90
>> ? cleanup_glue_dir+0x260/0x260
>> ? is_rtc_hctosys.isra.0+0xb9/0xf0
>> ? rtc_proc_show+0x440/0x440
>> ? rcu_read_lock_held_common+0xe/0xa0
>> ? rcu_read_lock_sched_held+0x62/0xe0
>> cdev_device_del+0x1a/0x80
>> devm_rtc_unregister_device+0x37/0x80
>> release_nodes+0xc3/0x3b0
>>
>> If cdev_device_add() fails, 'dev->p' is not set, it causes
>> null-ptr-deref when calling cdev_device_del(), return error
>> code when cdev_device_add() failed to fix this.
>>
>> Reported-by: Hulk Robot <[email protected]>
>> Fixes: 3068a254d5519 ("rtc: introduce new registration method")
>> Signed-off-by: Yang Yingliang <[email protected]>
>> ---
>> drivers/rtc/class.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
>> index 1f18c39a4b82..76422faee05b 100644
>> --- a/drivers/rtc/class.c
>> +++ b/drivers/rtc/class.c
>> @@ -399,12 +399,14 @@ int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc)
>> rtc_dev_prepare(rtc);
>>
>> err = cdev_device_add(&rtc->char_dev, &rtc->dev);
>> - if (err)
>> + if (err) {
>> dev_warn(rtc->dev.parent, "failed to add char device %d:%d\n",
>> MAJOR(rtc->dev.devt), rtc->id);
>> - else
>> + return err;
> As the checkpatch warning indicates, registering the character device as
> always been optional. d5ed9177f64fe95d9de79e6504d41612d9127e8a is the
> commit you want to fix.
So we can keep doing rtc register, even if cdev_device_add() failed ?

Thanks,
Yang
>
>> + } else {
>> dev_dbg(rtc->dev.parent, "char device (%d:%d)\n",
>> MAJOR(rtc->dev.devt), rtc->id);
>> + }
>>
>> rtc_proc_add_device(rtc);
>>
>> --
>> 2.25.1
>>

2021-10-11 13:29:52

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtc: class: return error code when cdev_device_add() failed

On 11/10/2021 17:40:34+0800, Yang Yingliang wrote:
>
> Hi,
>
> On 2021/10/11 16:36, Alexandre Belloni wrote:
> > On 11/10/2021 16:03:02+0800, Yang Yingliang wrote:
> > > I got a null-ptr-deref report when doing fault injection test:
> > >
> > > general protection fault, probably for non-canonical address 0xdffffc0000000022: 0000 [#1] SMP KASAN PTI
> > > KASAN: null-ptr-deref in range [0x0000000000000110-0x0000000000000117]
> > > CPU: 1 PID: 1028 Comm: 33 Not tainted 5.15.0-rc3-00111-gf5dad42ed4fe-dirty #481 2a70b3e6ca240b8638beac7ef491cce6183bbec7
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> > > RIP: 0010:device_del+0x132/0xdc0
> > > Code: 48 c1 ea 03 80 3c 02 00 0f 85 4f 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b 7b 48 4d 8d a7 10 01 00 00 4c 89 e2 48 c1 ea 03 <0f> b6 04 02 84 c0 74 06 0f 8e 7f 0a 00 00 45 0f b6 b7 10 01 00 00
> > > RSP: 0018:ffffc90002e876b8 EFLAGS: 00010206
> > > RAX: dffffc0000000000 RBX: ffff88801eb84000 RCX: ffffffff97227644
> > > RDX: 0000000000000022 RSI: ffff8880146a0000 RDI: 0000000000000002
> > > RBP: ffff88801eb84120 R08: fffffbfff349a60d R09: fffffbfff349a60d
> > > R10: ffffc90002e876b8 R11: fffffbfff349a60c R12: 0000000000000110
> > > R13: 0000000000000001 R14: ffffc90002e87848 R15: 0000000000000000
> > > FS: 00007fa514973500(0000) GS:ffff888104600000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 00007fa51474ccb0 CR3: 000000002bcb0001 CR4: 0000000000770ee0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > PKRU: 55555554
> > > Call Trace:
> > > ? write_comp_data+0x2a/0x90
> > > ? cleanup_glue_dir+0x260/0x260
> > > ? is_rtc_hctosys.isra.0+0xb9/0xf0
> > > ? rtc_proc_show+0x440/0x440
> > > ? rcu_read_lock_held_common+0xe/0xa0
> > > ? rcu_read_lock_sched_held+0x62/0xe0
> > > cdev_device_del+0x1a/0x80
> > > devm_rtc_unregister_device+0x37/0x80
> > > release_nodes+0xc3/0x3b0
> > >
> > > If cdev_device_add() fails, 'dev->p' is not set, it causes
> > > null-ptr-deref when calling cdev_device_del(), return error
> > > code when cdev_device_add() failed to fix this.
> > >
> > > Reported-by: Hulk Robot <[email protected]>
> > > Fixes: 3068a254d5519 ("rtc: introduce new registration method")
> > > Signed-off-by: Yang Yingliang <[email protected]>
> > > ---
> > > drivers/rtc/class.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> > > index 1f18c39a4b82..76422faee05b 100644
> > > --- a/drivers/rtc/class.c
> > > +++ b/drivers/rtc/class.c
> > > @@ -399,12 +399,14 @@ int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc)
> > > rtc_dev_prepare(rtc);
> > > err = cdev_device_add(&rtc->char_dev, &rtc->dev);
> > > - if (err)
> > > + if (err) {
> > > dev_warn(rtc->dev.parent, "failed to add char device %d:%d\n",
> > > MAJOR(rtc->dev.devt), rtc->id);
> > > - else
> > > + return err;
> > As the checkpatch warning indicates, registering the character device as
> > always been optional. d5ed9177f64fe95d9de79e6504d41612d9127e8a is the
> > commit you want to fix.
> So we can keep doing rtc register, even if cdev_device_add() failed ?
>

Yes, this has always been the case.


--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com