Re: [PATCH 1/2] staging: gpib: Fix NULL pointer dereference in detach

[Dan Carpenter]

On Sat, Jan 18, 2025 at 03:50:45PM +0100, Dave Penkler wrote:
> When the detach function is called after a failed attach
> the usb_dev initialization can cause a NULL pointer
> dereference. This happens when the usb device is not found
> in the attach procedure.
> 

What you're describing is true.  The ibonline() calls ->detach if
the ->attach() function fails:

drivers/staging/gpib/agilent_82357a/agilent_82357a.c
   230          retval = board->interface->attach(board, &board->config);
                                           ^^^^^^
   231          if (retval < 0) {
   232                  board->interface->detach(board);
                                          ^^^^^^
   233                  pr_err("gpib: interface attach failed\n");
   234                  return retval;
   235          }

But this is a class of bugs which is very typical when we free things
that are not allocated.  We would normally say, "don't detach things which
are not attached".  I would be surprised if this is the last bug caused by
this particular call to ->detach().

In fact, I notice that agilent_82357a_setup_urbs() has a
kfree(a_priv->interrupt_buffer) so that's a double free bug.  It's better
to have a system so that everyone knows exactly where to put the kfree().
Fortunately #SelfPromotion I have described such a system in my blog.

https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/

regards,
dan carpenter