linux-kernel - Re: [PATCH v2] staging: gpib: Fix device reference leak in fmh

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20250922133605.24777-1-make24@iscas.ac.cn>
Date: Mon, 22 Sep 2025 21:36:05 +0800
From: Ma Ke <make24@...as.ac.cn>
To: dan.carpenter@...aro.org
Cc: akpm@...ux-foundation.org,
	arnd@...db.de,
	dominik.karol.piatkowski@...tonmail.com,
	dpenkler@...il.com,
	gregkh@...uxfoundation.org,
	linux-kernel@...r.kernel.org,
	linux-staging@...ts.linux.dev,
	make24@...as.ac.cn,
	matchstick@...erthere.org,
	paul.retourne@...nge.fr,
	stable@...r.kernel.org
Subject: Re: [PATCH v2] staging: gpib: Fix device reference leak in fmh_gpib driver

On 9/22/25 19:28, Dan Carpenter wrote:
> On Mon, Sep 22, 2025 at 04:45:12PM +0800, Ma Ke wrote:
> > The fmh_gpib driver contains a device reference count leak in
> > fmh_gpib_attach_impl() where driver_find_device() increases the
> > reference count of the device by get_device() when matching but this
> > reference is not properly decreased. Add put_device() in
> > fmh_gpib_attach_impl() and add put_device() in fmh_gpib_detach(),
> > which ensures that the reference count of the device is correctly
> > managed.
> > 
> > Found by code review.
> > 
> > Cc: stable@...r.kernel.org
> > Fixes: 8e4841a0888c ("staging: gpib: Add Frank Mori Hess FPGA PCI GPIB driver")
> > Signed-off-by: Ma Ke <make24@...as.ac.cn>
> > ---
> > Changes in v2:
> > - modified the free operations as suggestions. Thanks for dan carpenter's instructions.
> > ---
> 
> Actually, it turns out that this isn't the right approach.  Sorry.
> This will introduce double frees.
> 
> The caller looks like this:
> 
> drivers/staging/gpib/common/iblib.c
>    204  int ibonline(struct gpib_board *board)
>    205  {
>    206          int retval;
>    207  
>    208          if (board->online)
>    209                  return -EBUSY;
>    210          if (!board->interface)
>    211                  return -ENODEV;
>    212          retval = gpib_allocate_board(board);
>    213          if (retval < 0)
>    214                  return retval;
>    215  
>    216          board->dev = NULL;
>    217          board->local_ppoll_mode = 0;
>    218          retval = board->interface->attach(board, &board->config);
>    219          if (retval < 0) {
>    220                  board->interface->detach(board);
> 
>    So if the attach() fails, we call ->detach() which works.
> 
>    221                  return retval;
>    222          }
> 
> It's weird because the fmh_gpib_pci_detach() function does have a
> put_device() in it:
> 
> 	if (board->dev)
> 		pci_dev_put(to_pci_dev(board->dev));
>                 ^^^^^^^^^^^
> 
> The detach functions are really similar...
> 
> regards,
> dan carpenter
Thank you very much for your patient reply. Indeed, I also carefully 
examined the implementations of the functions fmh_gpib_detach() and 
fmh_gpib_pci_detach(), and their code is indeed very similar. I also 
took note of the double free issue you mentioned. In patch v3, I will 
refer to the implementation of fmh_gpib_pci_detach() and only add 
put_device() in fmh_gpib_detach(). I will not make any modifications 
to other functions. Once again, thank you for your patient guidance, 
from which I have benefited greatly.

Best regards,

Ma Ke