linux-kernel - Re: [net,v2,2/2] net: liquidio: Initialize netdev pointer before queue setup

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: <aXeN1EsGgr_YQi8s@horms.kernel.org>
Date: Mon, 26 Jan 2026 15:52:52 +0000
From: Simon Horman <horms@...nel.org>
To: zilin@....edu.cn
Cc: vadim.fedorenko@...ux.dev, netdev@...r.kernel.org,
	andrew+netdev@...n.ch, davem@...emloft.net,
	kory.maincent@...tlin.com, edumazet@...gle.com, kuba@...nel.org,
	linux-kernel@...r.kernel.org, jianhao.xu@....edu.cn,
	pabeni@...hat.com, marco.crivellari@...e.com
Subject: Re: [net,v2,2/2] net: liquidio: Initialize netdev pointer before
 queue setup

On Mon, Jan 26, 2026 at 03:49:41PM +0000, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: liquidio: Initialize netdev pointer before queue setup
> 
> This patch moves the initialization of lio and props structures earlier
> in setup_nic_devices() so that props->netdev is set before calling
> netif_set_real_num_rx_queues() and netif_set_real_num_tx_queues(). The
> intent is to fix a memory leak where the allocated netdev would not be
> freed if queue setup fails.
> 
> > diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > index 3ba2806f5d1e..2383f0173a54 100644
> > --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > @@ -3505,6 +3505,16 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
> >  		 */
> >  		netdev->netdev_ops = &lionetdevops;
> >
> > +		lio = GET_LIO(netdev);
> > +
> > +		memset(lio, 0, sizeof(struct lio));
> > +
> > +		lio->ifidx = ifidx_or_pfnum;
> > +
> > +		props = &octeon_dev->props[i];
> > +		props->gmxport = resp->cfg_info.linfo.gmxport;
> > +		props->netdev = netdev;
> 
> Moving this initialization earlier fixes the memory leak, but now if
> netif_set_real_num_rx_queues() or netif_set_real_num_tx_queues() fails
> below, the cleanup path in liquidio_destroy_nic_device() will proceed
> further than before since props->netdev is no longer NULL.
> 
> Previously, when queue setup failed, liquidio_destroy_nic_device() would
> return early at the "if (!netdev)" check. Now it continues and calls
> cleanup_rx_oom_poll_fn().
> 
> In cleanup_rx_oom_poll_fn() (lio_core.c), the code does:
> 
>     struct lio *lio = GET_LIO(netdev);
>     struct octeon_device *oct = lio->oct_dev;
>     ...
>     for (q_no = 0; q_no < oct->num_oqs; q_no++) {
> 
> Since lio->oct_dev is not assigned until much later in setup_nic_devices()
> (around line 3602), it will be NULL after the memset above. Would this
> cause a NULL pointer dereference when cleanup_rx_oom_poll_fn() tries to
> access oct->num_oqs?
> 
> > +
> >  		retval = netif_set_real_num_rx_queues(netdev, num_oqueues);
> >  		if (retval) {
> >  			dev_err(&octeon_dev->pci_dev->dev,
> 
> [ ... ]

Strangely the AI review did not pick this up for v1.
(And neither did I.)