[PATCH] HID: intel-ish-hid: ipc: Fix dev_err usage with uninitialized dev->devc

[PATCH] HID: intel-ish-hid: ipc: Fix dev_err usage with uninitialized dev->devc

[Zhang, Lixu]

From: Zhang Lixu <lixu.zhang@intel.com>

The variable dev->devc in ish_dev_init was utilized by dev_err before it
was properly assigned. To rectify this, the assignment of dev->devc has
been moved to immediately follow memory allocation.

Without this change "(NULL device *)" is printed for device information.

Fixes: 8ae2f2b0a284 ("HID: intel-ish-hid: ipc: Fix potential use-after-free in work function")
Fixes: ae02e5d40d5f ("HID: intel-ish-hid: ipc layer")
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ipc/ipc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 7cc412798fdf..adce30f8ebff 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -948,6 +948,7 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev)
 	if (!dev)
 		return NULL;
 
+	dev->devc = &pdev->dev;
 	ishtp_device_init(dev);
 
 	init_waitqueue_head(&dev->wait_hw_ready);
@@ -983,7 +984,6 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev)
 	}
 
 	dev->ops = &ish_hw_ops;
-	dev->devc = &pdev->dev;
 	dev->mtu = IPC_PAYLOAD_SIZE - sizeof(struct ishtp_msg_hdr);
 	return dev;
 }
-- 
2.40.1

Re: [PATCH] HID: intel-ish-hid: ipc: Fix dev_err usage with uninitialized dev->devc

[srinivas pandruvada]

On Wed, 2024-03-06 at 00:44 +0000, Zhang, Lixu wrote:
> From: Zhang Lixu <lixu.zhang@intel.com>
> 
> The variable dev->devc in ish_dev_init was utilized by dev_err before
> it
> was properly assigned. To rectify this, the assignment of dev->devc
> has
> been moved to immediately follow memory allocation.
> 
> Without this change "(NULL device *)" is printed for device
> information.
> 
> Fixes: 8ae2f2b0a284 ("HID: intel-ish-hid: ipc: Fix potential use-
> after-free in work function")
> Fixes: ae02e5d40d5f ("HID: intel-ish-hid: ipc layer")
> Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
This is not an important change to submit to rc cycle.
This is just a logging issue.

Thanks,
Srinivas

>  drivers/hid/intel-ish-hid/ipc/ipc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-
> ish-hid/ipc/ipc.c
> index 7cc412798fdf..adce30f8ebff 100644
> --- a/drivers/hid/intel-ish-hid/ipc/ipc.c
> +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
> @@ -948,6 +948,7 @@ struct ishtp_device *ish_dev_init(struct pci_dev
> *pdev)
>         if (!dev)
>                 return NULL;
>  
> +       dev->devc = &pdev->dev;
>         ishtp_device_init(dev);
>  
>         init_waitqueue_head(&dev->wait_hw_ready);
> @@ -983,7 +984,6 @@ struct ishtp_device *ish_dev_init(struct pci_dev
> *pdev)
>         }
>  
>         dev->ops = &ish_hw_ops;
> -       dev->devc = &pdev->dev;
>         dev->mtu = IPC_PAYLOAD_SIZE - sizeof(struct ishtp_msg_hdr);
>         return dev;
>  }

Re: [PATCH] HID: intel-ish-hid: ipc: Fix dev_err usage with uninitialized dev->devc

[Jiri Kosina]

On Tue, 5 Mar 2024, srinivas pandruvada wrote:

> > The variable dev->devc in ish_dev_init was utilized by dev_err before 
> > it was properly assigned. To rectify this, the assignment of dev->devc 
> > has been moved to immediately follow memory allocation.
> > 
> > Without this change "(NULL device *)" is printed for device
> > information.
> > 
> > Fixes: 8ae2f2b0a284 ("HID: intel-ish-hid: ipc: Fix potential use-
> > after-free in work function")
> > Fixes: ae02e5d40d5f ("HID: intel-ish-hid: ipc layer")
> > Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > ---
> This is not an important change to submit to rc cycle.
> This is just a logging issue.

Agreed about the severity. However it fixes a user-visible issue and 
doesn't have a potential to cause any regression, so I have just queued it 
in hid.git#for-6.9/upstream-fixes nevertheless.

Thanks,

-- 
Jiri Kosina
SUSE Labs