.../gpib/agilent_82357a/agilent_82357a.c | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
This commit fixes a dereference before null check issue discovered by
Coverity (CID 1601566).
The check at line 1450 suggests that a_priv can be NULL, however it has
been dereferenced before, in the interface_to_usbdev() call.
After a discussion, the NULL check is useless because private data is allocated
in attach routine and freed in detach routine, so in detach routine the pointer
has to be not NULL for sure.
Signed-off-by: Paolo Perego <pperego@suse.de>
Fixes: Removes a useless NULL check
---
v2:
* After a discussion, we agreed the NULL check is useless because private data
is freed in detach routine and allocated while attaching. For such a reason,
the pointer has to be not NULL while detaching the device
.../gpib/agilent_82357a/agilent_82357a.c | 20 +++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/gpib/agilent_82357a/agilent_82357a.c b/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
index bf05fb4a736b..e940429dea19 100644
--- a/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
+++ b/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
@@ -1447,17 +1447,17 @@ static void agilent_82357a_detach(gpib_board_t *board)
a_priv = board->private_data;
usb_dev = interface_to_usbdev(a_priv->bus_interface);
- if (a_priv) {
- if (a_priv->bus_interface) {
- agilent_82357a_go_idle(board);
- usb_set_intfdata(a_priv->bus_interface, NULL);
- }
- mutex_lock(&a_priv->control_alloc_lock);
- mutex_lock(&a_priv->bulk_alloc_lock);
- mutex_lock(&a_priv->interrupt_alloc_lock);
- agilent_82357a_cleanup_urbs(a_priv);
- agilent_82357a_free_private(a_priv);
+
+ if (a_priv->bus_interface) {
+ agilent_82357a_go_idle(board);
+ usb_set_intfdata(a_priv->bus_interface, NULL);
}
+
+ mutex_lock(&a_priv->control_alloc_lock);
+ mutex_lock(&a_priv->bulk_alloc_lock);
+ mutex_lock(&a_priv->interrupt_alloc_lock);
+ agilent_82357a_cleanup_urbs(a_priv);
+ agilent_82357a_free_private(a_priv);
dev_info(&usb_dev->dev, "%s: detached\n", __func__);
mutex_unlock(&agilent_82357a_hotplug_lock);
}
--
2.47.0
On Thu, Nov 21, 2024 at 04:22:21PM +0100, Paolo Perego wrote: > This commit fixes a dereference before null check issue discovered by > Coverity (CID 1601566). > > The check at line 1450 suggests that a_priv can be NULL, however it has > been dereferenced before, in the interface_to_usbdev() call. > > After a discussion, the NULL check is useless because private data is allocated > in attach routine and freed in detach routine, so in detach routine the pointer > has to be not NULL for sure. > > Signed-off-by: Paolo Perego <pperego@suse.de> > Fixes: Removes a useless NULL check This isn't how the Fixes tag works. It's supposed to refer to the patch which introduces the static checker warning. Fixes: fbae7090f30c ("staging: gpib: Update messaging and usb_device refs in agilent_usb") The patch has a couple checkpatch issues as well. Please, fix those up and resend. regards, dan carpenter
© 2016 - 2024 Red Hat, Inc.