On Thu, Nov 21, 2024 at 10:37:30AM GMT, Dan Carpenter wrote: > On Wed, Nov 20, 2024 at 08:54:16PM +0100, Kees Bakker wrote: > > Op 20-11-2024 om 18:04 schreef Dan Carpenter: > > > On Wed, Nov 20, 2024 at 03:46:53PM +0100, Paolo Perego wrote: > > > > This commit fixes a dereference before null check issue discovered by > > > > Coverity (CID 1601566). > > > > > > > > The check ad line 1450 suggests that a_priv can be NULL, however it has > > > > been derefenced before, in the interface_to_usbdev() call. > > > > > > > > Signed-off-by: Paolo Perego <pperego@xxxxxxx> > > > > --- > > > You need a Fixes tag. But I'm pretty sure the correct fix is to remove the NULL > > > check. > > In the whole agilent_82357a.c module there is no consistency whether > > board->private_data needs to be checked for a NULL or not. > > > > If Dan is correct then it is better to drop the NULL check, not only here > > but in a few more places as well. There are at least 10 functions were > > there is no NULL check for private_data. > > > > Run this command and you'll see what I mean > > git grep -3 -e '->private_data' -- drivers/staging/gpib/agilent_82357a > > > > I had looked at similar issue in a different driver: > https://lore.kernel.org/all/2d99b7a6-f427-4d54-bde7-fb3df5e19e53@stanley.mountain/ > > Here the NULL check we are discussing is the same thing. The private data is > allocated in attach() and freed in detach(). The detach has no need to check > for NULL because we can't detach something which isn't attached. > > The other NULL checks are in agilent_82357a_driver_disconnect(), > agilent_82357a_driver_suspend() and agilent_82357a_driver_resume(). And there > the NULL checks are required because it could happen when the driver isn't > attached. > > I also did a quick glance through to see if any of the functions which didn't > check for NULL should get a NULL check but they all seemed okay because either > the board was attached or the caller had a NULL check. > Hi all and thanks for the fruitful discussion. > So I think we can just remove this one NULL check and everything else makes > sense. Please, apologise if I'm too newbie here to understand next step on my own. Am I asked to do something, to submit a V2 with the correct Tag or the patch is good as is? ( No pressure to be accepted, it's just my willing to understand and go into the process :-) ) Thanks Paolo -- (*_ Paolo Perego @thesp0nge //\ Software security engineer suse.com V_/_ 0A1A 2003 9AE0 B09C 51A4 7ACD FC0D CEA6 0806 294B
Attachment:
signature.asc
Description: PGP signature
