* [PATCH] hwmon: (nct6775) Fix crash in clear_caseopen
@ 2022-01-24 2:32 Guenter Roeck
2022-01-24 22:21 ` Denis Pauk
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2022-01-24 2:32 UTC (permalink / raw)
To: Hardware Monitoring
Cc: Jean Delvare, Guenter Roeck, Denis Pauk, Bernhard Seibold,
Paweł Marciniak
Paweł Marciniak reports the following crash, observed when clearing
the chassis intrusion alarm.
BUG: kernel NULL pointer dereference, address: 0000000000000028
PGD 0 P4D 0
Oops: 0000 [#1] PREEMPT SMP PTI
CPU: 3 PID: 4815 Comm: bash Tainted: G S 5.16.2-200.fc35.x86_64 #1
Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z97 Extreme4, BIOS P2.60A 05/03/2018
RIP: 0010:clear_caseopen+0x5a/0x120 [nct6775]
Code: 68 70 e8 e9 32 b1 e3 85 c0 0f 85 d2 00 00 00 48 83 7c 24 ...
RSP: 0018:ffffabcb02803dd8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: ffff8e8808192880 RSI: 0000000000000000 RDI: ffff8e87c7509a68
RBP: 0000000000000000 R08: 0000000000000001 R09: 000000000000000a
R10: 000000000000000a R11: f000000000000000 R12: 000000000000001f
R13: ffff8e87c7509828 R14: ffff8e87c7509a68 R15: ffff8e88494527a0
FS: 00007f4db9151740(0000) GS:ffff8e8ebfec0000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000028 CR3: 0000000166b66001 CR4: 00000000001706e0
Call Trace:
<TASK>
kernfs_fop_write_iter+0x11c/0x1b0
new_sync_write+0x10b/0x180
vfs_write+0x209/0x2a0
ksys_write+0x4f/0xc0
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x44/0xae
The problem is that the device passed to clear_caseopen() is the hwmon
device, not the platform device, and the platform data is not set in the
hwmon device. Store the pointer to sio_data in struct nct6775_data and
get if from there if needed.
Fixes: 2e7b9886968b ("hwmon: (nct6775) Use superio_*() function pointers in sio_data.")
Cc: Denis Pauk <pauk.denis@gmail.com>
Cc: Bernhard Seibold <mail@bernhard-seibold.de>
Reported-by: Paweł Marciniak <pmarciniak@lodz.home.pl>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/nct6775.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
index fd3f91cb01c6..098d12b9ecda 100644
--- a/drivers/hwmon/nct6775.c
+++ b/drivers/hwmon/nct6775.c
@@ -1175,7 +1175,7 @@ static inline u8 in_to_reg(u32 val, u8 nr)
struct nct6775_data {
int addr; /* IO base of hw monitor block */
- int sioreg; /* SIO register address */
+ struct nct6775_sio_data *sio_data;
enum kinds kind;
const char *name;
@@ -3559,7 +3559,7 @@ clear_caseopen(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct nct6775_data *data = dev_get_drvdata(dev);
- struct nct6775_sio_data *sio_data = dev_get_platdata(dev);
+ struct nct6775_sio_data *sio_data = data->sio_data;
int nr = to_sensor_dev_attr(attr)->index - INTRUSION_ALARM_BASE;
unsigned long val;
u8 reg;
@@ -3967,7 +3967,7 @@ static int nct6775_probe(struct platform_device *pdev)
return -ENOMEM;
data->kind = sio_data->kind;
- data->sioreg = sio_data->sioreg;
+ data->sio_data = sio_data;
if (sio_data->access == access_direct) {
data->addr = res->start;
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] hwmon: (nct6775) Fix crash in clear_caseopen
2022-01-24 2:32 [PATCH] hwmon: (nct6775) Fix crash in clear_caseopen Guenter Roeck
@ 2022-01-24 22:21 ` Denis Pauk
2022-01-24 22:32 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Denis Pauk @ 2022-01-24 22:21 UTC (permalink / raw)
To: Guenter Roeck
Cc: Hardware Monitoring, Jean Delvare, Bernhard Seibold,
Paweł Marciniak
On Sun, 23 Jan 2022 18:32:48 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> Paweł Marciniak reports the following crash, observed when clearing
> the chassis intrusion alarm.
>
> BUG: kernel NULL pointer dereference, address: 0000000000000028
> PGD 0 P4D 0
> Oops: 0000 [#1] PREEMPT SMP PTI
> CPU: 3 PID: 4815 Comm: bash Tainted: G S
> 5.16.2-200.fc35.x86_64 #1 Hardware name: To Be Filled By O.E.M. To Be
> Filled By O.E.M./Z97 Extreme4, BIOS P2.60A 05/03/2018 RIP:
> 0010:clear_caseopen+0x5a/0x120 [nct6775] Code: 68 70 e8 e9 32 b1 e3
> 85 c0 0f 85 d2 00 00 00 48 83 7c 24 ... RSP: 0018:ffffabcb02803dd8
> EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000002 RCX:
> 0000000000000000 RDX: ffff8e8808192880 RSI: 0000000000000000 RDI:
> ffff8e87c7509a68 RBP: 0000000000000000 R08: 0000000000000001 R09:
> 000000000000000a R10: 000000000000000a R11: f000000000000000 R12:
> 000000000000001f R13: ffff8e87c7509828 R14: ffff8e87c7509a68 R15:
> ffff8e88494527a0 FS: 00007f4db9151740(0000)
> GS:ffff8e8ebfec0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000
> ES: 0000 CR0: 0000000080050033 CR2: 0000000000000028 CR3:
> 0000000166b66001 CR4: 00000000001706e0 Call Trace:
> <TASK>
> kernfs_fop_write_iter+0x11c/0x1b0
> new_sync_write+0x10b/0x180
> vfs_write+0x209/0x2a0
> ksys_write+0x4f/0xc0
> do_syscall_64+0x3b/0x90
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> The problem is that the device passed to clear_caseopen() is the hwmon
> device, not the platform device, and the platform data is not set in
> the hwmon device. Store the pointer to sio_data in struct
> nct6775_data and get if from there if needed.
>
> Fixes: 2e7b9886968b ("hwmon: (nct6775) Use superio_*() function
> pointers in sio_data.") Cc: Denis Pauk <pauk.denis@gmail.com>
> Cc: Bernhard Seibold <mail@bernhard-seibold.de>
> Reported-by: Paweł Marciniak <pmarciniak@lodz.home.pl>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/nct6775.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
> index fd3f91cb01c6..098d12b9ecda 100644
> --- a/drivers/hwmon/nct6775.c
> +++ b/drivers/hwmon/nct6775.c
> @@ -1175,7 +1175,7 @@ static inline u8 in_to_reg(u32 val, u8 nr)
>
> struct nct6775_data {
> int addr; /* IO base of hw monitor block */
> - int sioreg; /* SIO register address */
> + struct nct6775_sio_data *sio_data;
> enum kinds kind;
What do you think about remove kind field also from struct
nct6775_data? Code has copy of this field inside of the struct
nct6775_sio_data.
> const char *name;
>
> @@ -3559,7 +3559,7 @@ clear_caseopen(struct device *dev, struct
> device_attribute *attr, const char *buf, size_t count)
> {
> struct nct6775_data *data = dev_get_drvdata(dev);
> - struct nct6775_sio_data *sio_data = dev_get_platdata(dev);
> + struct nct6775_sio_data *sio_data = data->sio_data;
> int nr = to_sensor_dev_attr(attr)->index -
> INTRUSION_ALARM_BASE; unsigned long val;
> u8 reg;
> @@ -3967,7 +3967,7 @@ static int nct6775_probe(struct platform_device
> *pdev) return -ENOMEM;
>
> data->kind = sio_data->kind;
> - data->sioreg = sio_data->sioreg;
> + data->sio_data = sio_data;
>
> if (sio_data->access == access_direct) {
> data->addr = res->start;
Tested-by: Denis Pauk <pauk.denis@gmail.com>
Best regards,
Denis.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] hwmon: (nct6775) Fix crash in clear_caseopen
2022-01-24 22:21 ` Denis Pauk
@ 2022-01-24 22:32 ` Guenter Roeck
2022-01-25 21:12 ` Denis Pauk
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2022-01-24 22:32 UTC (permalink / raw)
To: Denis Pauk
Cc: Hardware Monitoring, Jean Delvare, Bernhard Seibold,
Paweł Marciniak
On 1/24/22 14:21, Denis Pauk wrote:
> On Sun, 23 Jan 2022 18:32:48 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
>> Paweł Marciniak reports the following crash, observed when clearing
>> the chassis intrusion alarm.
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000028
>> PGD 0 P4D 0
>> Oops: 0000 [#1] PREEMPT SMP PTI
>> CPU: 3 PID: 4815 Comm: bash Tainted: G S
>> 5.16.2-200.fc35.x86_64 #1 Hardware name: To Be Filled By O.E.M. To Be
>> Filled By O.E.M./Z97 Extreme4, BIOS P2.60A 05/03/2018 RIP:
>> 0010:clear_caseopen+0x5a/0x120 [nct6775] Code: 68 70 e8 e9 32 b1 e3
>> 85 c0 0f 85 d2 00 00 00 48 83 7c 24 ... RSP: 0018:ffffabcb02803dd8
>> EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000002 RCX:
>> 0000000000000000 RDX: ffff8e8808192880 RSI: 0000000000000000 RDI:
>> ffff8e87c7509a68 RBP: 0000000000000000 R08: 0000000000000001 R09:
>> 000000000000000a R10: 000000000000000a R11: f000000000000000 R12:
>> 000000000000001f R13: ffff8e87c7509828 R14: ffff8e87c7509a68 R15:
>> ffff8e88494527a0 FS: 00007f4db9151740(0000)
>> GS:ffff8e8ebfec0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000
>> ES: 0000 CR0: 0000000080050033 CR2: 0000000000000028 CR3:
>> 0000000166b66001 CR4: 00000000001706e0 Call Trace:
>> <TASK>
>> kernfs_fop_write_iter+0x11c/0x1b0
>> new_sync_write+0x10b/0x180
>> vfs_write+0x209/0x2a0
>> ksys_write+0x4f/0xc0
>> do_syscall_64+0x3b/0x90
>> entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> The problem is that the device passed to clear_caseopen() is the hwmon
>> device, not the platform device, and the platform data is not set in
>> the hwmon device. Store the pointer to sio_data in struct
>> nct6775_data and get if from there if needed.
>>
>> Fixes: 2e7b9886968b ("hwmon: (nct6775) Use superio_*() function
>> pointers in sio_data.") Cc: Denis Pauk <pauk.denis@gmail.com>
>> Cc: Bernhard Seibold <mail@bernhard-seibold.de>
>> Reported-by: Paweł Marciniak <pmarciniak@lodz.home.pl>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/hwmon/nct6775.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
>> index fd3f91cb01c6..098d12b9ecda 100644
>> --- a/drivers/hwmon/nct6775.c
>> +++ b/drivers/hwmon/nct6775.c
>> @@ -1175,7 +1175,7 @@ static inline u8 in_to_reg(u32 val, u8 nr)
>>
>> struct nct6775_data {
>> int addr; /* IO base of hw monitor block */
>> - int sioreg; /* SIO register address */
>> + struct nct6775_sio_data *sio_data;
>> enum kinds kind;
> What do you think about remove kind field also from struct
> nct6775_data? Code has copy of this field inside of the struct
> nct6775_sio_data.
I thought about it, but it would add a level of indirection to
each access, and it is used for each register access (through
is_word_sized). That would make the change pretty expensive
in terms of runtime cost.
Guenter
>> const char *name;
>>
>> @@ -3559,7 +3559,7 @@ clear_caseopen(struct device *dev, struct
>> device_attribute *attr, const char *buf, size_t count)
>> {
>> struct nct6775_data *data = dev_get_drvdata(dev);
>> - struct nct6775_sio_data *sio_data = dev_get_platdata(dev);
>> + struct nct6775_sio_data *sio_data = data->sio_data;
>> int nr = to_sensor_dev_attr(attr)->index -
>> INTRUSION_ALARM_BASE; unsigned long val;
>> u8 reg;
>> @@ -3967,7 +3967,7 @@ static int nct6775_probe(struct platform_device
>> *pdev) return -ENOMEM;
>>
>> data->kind = sio_data->kind;
>> - data->sioreg = sio_data->sioreg;
>> + data->sio_data = sio_data;
>>
>> if (sio_data->access == access_direct) {
>> data->addr = res->start;
>
> Tested-by: Denis Pauk <pauk.denis@gmail.com>
>
> Best regards,
> Denis.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] hwmon: (nct6775) Fix crash in clear_caseopen
2022-01-24 22:32 ` Guenter Roeck
@ 2022-01-25 21:12 ` Denis Pauk
0 siblings, 0 replies; 4+ messages in thread
From: Denis Pauk @ 2022-01-25 21:12 UTC (permalink / raw)
To: Guenter Roeck
Cc: Hardware Monitoring, Jean Delvare, Bernhard Seibold,
Paweł Marciniak
On Mon, 24 Jan 2022 14:32:26 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> On 1/24/22 14:21, Denis Pauk wrote:
> > On Sun, 23 Jan 2022 18:32:48 -0800
> > Guenter Roeck <linux@roeck-us.net> wrote:
> >
> >> Paweł Marciniak reports the following crash, observed when clearing
> >> the chassis intrusion alarm.
> >>
> >> BUG: kernel NULL pointer dereference, address: 0000000000000028
> >> PGD 0 P4D 0
> >> Oops: 0000 [#1] PREEMPT SMP PTI
> >> CPU: 3 PID: 4815 Comm: bash Tainted: G S
> >> 5.16.2-200.fc35.x86_64 #1 Hardware name: To Be Filled By O.E.M. To
> >> Be Filled By O.E.M./Z97 Extreme4, BIOS P2.60A 05/03/2018 RIP:
> >> 0010:clear_caseopen+0x5a/0x120 [nct6775] Code: 68 70 e8 e9 32 b1 e3
> >> 85 c0 0f 85 d2 00 00 00 48 83 7c 24 ... RSP: 0018:ffffabcb02803dd8
> >> EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000002 RCX:
> >> 0000000000000000 RDX: ffff8e8808192880 RSI: 0000000000000000 RDI:
> >> ffff8e87c7509a68 RBP: 0000000000000000 R08: 0000000000000001 R09:
> >> 000000000000000a R10: 000000000000000a R11: f000000000000000 R12:
> >> 000000000000001f R13: ffff8e87c7509828 R14: ffff8e87c7509a68 R15:
> >> ffff8e88494527a0 FS: 00007f4db9151740(0000)
> >> GS:ffff8e8ebfec0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000
> >> ES: 0000 CR0: 0000000080050033 CR2: 0000000000000028 CR3:
> >> 0000000166b66001 CR4: 00000000001706e0 Call Trace:
> >> <TASK>
> >> kernfs_fop_write_iter+0x11c/0x1b0
> >> new_sync_write+0x10b/0x180
> >> vfs_write+0x209/0x2a0
> >> ksys_write+0x4f/0xc0
> >> do_syscall_64+0x3b/0x90
> >> entry_SYSCALL_64_after_hwframe+0x44/0xae
> >>
> >> The problem is that the device passed to clear_caseopen() is the
> >> hwmon device, not the platform device, and the platform data is
> >> not set in the hwmon device. Store the pointer to sio_data in
> >> struct nct6775_data and get if from there if needed.
> >>
> >> Fixes: 2e7b9886968b ("hwmon: (nct6775) Use superio_*() function
> >> pointers in sio_data.") Cc: Denis Pauk <pauk.denis@gmail.com>
> >> Cc: Bernhard Seibold <mail@bernhard-seibold.de>
> >> Reported-by: Paweł Marciniak <pmarciniak@lodz.home.pl>
> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >> ---
> >> drivers/hwmon/nct6775.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
> >> index fd3f91cb01c6..098d12b9ecda 100644
> >> --- a/drivers/hwmon/nct6775.c
> >> +++ b/drivers/hwmon/nct6775.c
> >> @@ -1175,7 +1175,7 @@ static inline u8 in_to_reg(u32 val, u8 nr)
> >>
> >> struct nct6775_data {
> >> int addr; /* IO base of hw monitor block */
> >> - int sioreg; /* SIO register address */
> >> + struct nct6775_sio_data *sio_data;
> >> enum kinds kind;
> > What do you think about remove kind field also from struct
> > nct6775_data? Code has copy of this field inside of the struct
> > nct6775_sio_data.
>
> I thought about it, but it would add a level of indirection to
> each access, and it is used for each register access (through
> is_word_sized). That would make the change pretty expensive
> in terms of runtime cost.
>
I see, thank you.
> Guenter
>
> >> const char *name;
> >>
> >> @@ -3559,7 +3559,7 @@ clear_caseopen(struct device *dev, struct
> >> device_attribute *attr, const char *buf, size_t count)
> >> {
> >> struct nct6775_data *data = dev_get_drvdata(dev);
> >> - struct nct6775_sio_data *sio_data = dev_get_platdata(dev);
> >> + struct nct6775_sio_data *sio_data = data->sio_data;
> >> int nr = to_sensor_dev_attr(attr)->index -
> >> INTRUSION_ALARM_BASE; unsigned long val;
> >> u8 reg;
> >> @@ -3967,7 +3967,7 @@ static int nct6775_probe(struct
> >> platform_device *pdev) return -ENOMEM;
> >>
> >> data->kind = sio_data->kind;
> >> - data->sioreg = sio_data->sioreg;
> >> + data->sio_data = sio_data;
> >>
> >> if (sio_data->access == access_direct) {
> >> data->addr = res->start;
> >
> > Tested-by: Denis Pauk <pauk.denis@gmail.com>
> >
> > Best regards,
> > Denis.
>
Best regards,
Denis.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-25 21:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-24 2:32 [PATCH] hwmon: (nct6775) Fix crash in clear_caseopen Guenter Roeck
2022-01-24 22:21 ` Denis Pauk
2022-01-24 22:32 ` Guenter Roeck
2022-01-25 21:12 ` Denis Pauk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox