Re: [PATCH] hwmon: (nct6775) Fix crash in clear_caseopen

[Denis Pauk <pauk.denis@gmail.com>]

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.