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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aNEy7LsnKa50Pq3c@stanley.mountain>
Date: Mon, 22 Sep 2025 14:28:44 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Ma Ke <make24@...as.ac.cn>
Cc: dpenkler@...il.com, gregkh@...uxfoundation.org,
	matchstick@...erthere.org, dominik.karol.piatkowski@...tonmail.com,
	arnd@...db.de, nichen@...as.ac.cn, paul.retourne@...nge.fr,
	linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] staging: gpib: Fix device reference leak in fmh_gpib
 driver

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