[PATCH net v2 0/2] net: liquidio: Fix memory leaks in setup_nic_devices()

[PATCH net v2 0/2] net: liquidio: Fix memory leaks in setup_nic_devices()

[Zilin Guan]

Patch 1 fixes an off-by-one error in the cleanup loop. Using a do-while 
loop ensures the current device index is also cleaned up.

Patch 2 moves the initialization of oct->props[i].netdev before queue 
setup calls. This ensures that if queue setup fails, the cleanup function 
can find the allocated netdev.

Signed-off-by: Zilin Guan <zilin@seu.edu.cn>

Changes in v2:
- Add patch 1 to fix an off-by-one error in the error handling loop logic.

Zilin Guan (2):
  net: liquidio: Fix off-by-one error in setup_nic_devices() cleanup
  net: liquidio: Initialize netdev pointer before queue setup

 .../net/ethernet/cavium/liquidio/lio_main.c   | 24 +++++++++----------
 .../ethernet/cavium/liquidio/lio_vf_main.c    |  4 ++--
 2 files changed, 14 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH net v2 1/2] net: liquidio: Fix off-by-one error in setup_nic_devices() cleanup

[Zilin Guan]

In setup_nic_devices, the initialization loop jumps to the label
setup_nic_dev_free on failure. The current error handling path uses
a while loop that decrements the device index i. This logic skips
the release of resources for the device at the current index i where
the failure occurred.

Use a do-while loop instead. This guarantees that
liquidio_destroy_nic_device() is called for the failing device index i
and continues for all previously initialized devices.

Compile tested only. Issue found using code review.

Fixes: f21fb3ed364b ("Add support of Cavium Liquidio ethernet adapters")
Fixes: 846b46873eeb ("liquidio CN23XX: VF offload features")
Suggested-by: Simon Horman <horms@kernel.org>
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
 drivers/net/ethernet/cavium/liquidio/lio_main.c    | 4 ++--
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 0732440eeacd..3ba2806f5d1e 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -3765,11 +3765,11 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
 
 setup_nic_dev_free:
 
-	while (i--) {
+	do {
 		dev_err(&octeon_dev->pci_dev->dev,
 			"NIC ifidx:%d Setup failed\n", i);
 		liquidio_destroy_nic_device(octeon_dev, i);
-	}
+	} while (i--);
 
 setup_nic_dev_done:
 
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index e02942dbbcce..43c595f3b84e 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -2212,11 +2212,11 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
 
 setup_nic_dev_free:
 
-	while (i--) {
+	do {
 		dev_err(&octeon_dev->pci_dev->dev,
 			"NIC ifidx:%d Setup failed\n", i);
 		liquidio_destroy_nic_device(octeon_dev, i);
-	}
+	} while (i--);
 
 setup_nic_dev_done:
 
-- 
2.34.1

[PATCH net v2 2/2] net: liquidio: Initialize netdev pointer before queue setup

[Zilin Guan]

In setup_nic_devices(), the netdev is allocated using alloc_etherdev_mq().
However, the pointer to this structure is stored in oct->props[i].netdev
only after the calls to netif_set_real_num_rx_queues() and
netif_set_real_num_tx_queues().

If either of these functions fails, setup_nic_devices() returns an error
without freeing the allocated netdev. Since oct->props[i].netdev is still
NULL at this point, the cleanup function liquidio_destroy_nic_device()
will fail to find and free the netdev, resulting in a memory leak.

Fix this by initializing oct->props[i].netdev before calling the queue
setup functions. This ensures that the netdev is properly accessible for
cleanup in case of errors.

Compile tested only. Issue found using a prototype static analysis tool
and code review.

Fixes: c33c997346c3 ("liquidio: enhanced ethtool --set-channels feature")
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
 .../net/ethernet/cavium/liquidio/lio_main.c   | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

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;
+
 		retval = netif_set_real_num_rx_queues(netdev, num_oqueues);
 		if (retval) {
 			dev_err(&octeon_dev->pci_dev->dev,
@@ -3521,16 +3531,6 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
 			goto setup_nic_dev_free;
 		}
 
-		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;
-
 		lio->linfo.num_rxpciq = num_oqueues;
 		lio->linfo.num_txpciq = num_iqueues;
 		for (j = 0; j < num_oqueues; j++) {
-- 
2.34.1

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

[Simon Horman]

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,

[ ... ]
-- 
pw-bot: cr

Re: [net,v2,1/2] net: liquidio: Fix off-by-one error in setup_nic_devices() cleanup

[Simon Horman]

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: Fix off-by-one error in setup_nic_devices() cleanup

This patch changes the cleanup loop from while(i--) to do{}while(i--)
to ensure that when a device fails to initialize, it gets cleaned up
along with all previously initialized devices.

> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> index 0732440eeacd..3ba2806f5d1e 100644
> --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
> +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> @@ -3765,11 +3765,11 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
>
>  setup_nic_dev_free:
>
> -	while (i--) {
> +	do {
>  		dev_err(&octeon_dev->pci_dev->dev,
>  			"NIC ifidx:%d Setup failed\n", i);
>  		liquidio_destroy_nic_device(octeon_dev, i);
> -	}
> +	} while (i--);

In lio_main.c, setup_nic_devices() can also reach setup_nic_dev_free from
the devlink_alloc() failure path after the main for loop completes. At that
point i equals ifcount (the loop counter has been incremented past the last
valid device).

With the do-while change, the first iteration will call
liquidio_destroy_nic_device(octeon_dev, ifcount) for a device index that
was never initialized. The function has a guard for NULL netdev so it will
not crash, but it will print a spurious error message about a device that
never existed.

The fix works correctly for failures within the loop (which is the case
described in the commit message), but is there a way to handle the
post-loop devlink_alloc() failure case without printing the spurious
error message?

[ ... ]
-- 
pw-bot: cr

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

[Simon Horman]

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.)

Re: [net,v2,1/2] net: liquidio: Fix off-by-one error in setup_nic_devices() cleanup

[Simon Horman]

On Mon, Jan 26, 2026 at 03:50:39PM +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: Fix off-by-one error in setup_nic_devices() cleanup
> 
> This patch changes the cleanup loop from while(i--) to do{}while(i--)
> to ensure that when a device fails to initialize, it gets cleaned up
> along with all previously initialized devices.
> 
> > diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > index 0732440eeacd..3ba2806f5d1e 100644
> > --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > @@ -3765,11 +3765,11 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
> >
> >  setup_nic_dev_free:
> >
> > -	while (i--) {
> > +	do {
> >  		dev_err(&octeon_dev->pci_dev->dev,
> >  			"NIC ifidx:%d Setup failed\n", i);
> >  		liquidio_destroy_nic_device(octeon_dev, i);
> > -	}
> > +	} while (i--);
> 
> In lio_main.c, setup_nic_devices() can also reach setup_nic_dev_free from
> the devlink_alloc() failure path after the main for loop completes. At that
> point i equals ifcount (the loop counter has been incremented past the last
> valid device).
> 
> With the do-while change, the first iteration will call
> liquidio_destroy_nic_device(octeon_dev, ifcount) for a device index that
> was never initialized. The function has a guard for NULL netdev so it will
> not crash, but it will print a spurious error message about a device that
> never existed.
> 
> The fix works correctly for failures within the loop (which is the case
> described in the commit message), but is there a way to handle the
> post-loop devlink_alloc() failure case without printing the spurious
> error message?

Sorry for not realising this when I made this suggestion in
my review of v1.

Also, I would suggest splitting this patch in two: one patch
per driver.

Re: [net,v2,1/2] net: liquidio: Fix off-by-one error in setup_nic_devices() cleanup

[Zilin Guan]

On Mon, Jan 26, 2026 at 03:56:07PM +0000, Simon Horman wrote:
> On Mon, Jan 26, 2026 at 03:50:39PM +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: Fix off-by-one error in setup_nic_devices() cleanup
> > 
> > This patch changes the cleanup loop from while(i--) to do{}while(i--)
> > to ensure that when a device fails to initialize, it gets cleaned up
> > along with all previously initialized devices.
> > 
> > > diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > > index 0732440eeacd..3ba2806f5d1e 100644
> > > --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > > +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > > @@ -3765,11 +3765,11 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
> > >
> > >  setup_nic_dev_free:
> > >
> > > -	while (i--) {
> > > +	do {
> > >  		dev_err(&octeon_dev->pci_dev->dev,
> > >  			"NIC ifidx:%d Setup failed\n", i);
> > >  		liquidio_destroy_nic_device(octeon_dev, i);
> > > -	}
> > > +	} while (i--);
> > 
> > In lio_main.c, setup_nic_devices() can also reach setup_nic_dev_free from
> > the devlink_alloc() failure path after the main for loop completes. At that
> > point i equals ifcount (the loop counter has been incremented past the last
> > valid device).
> > 
> > With the do-while change, the first iteration will call
> > liquidio_destroy_nic_device(octeon_dev, ifcount) for a device index that
> > was never initialized. The function has a guard for NULL netdev so it will
> > not crash, but it will print a spurious error message about a device that
> > never existed.
> > 
> > The fix works correctly for failures within the loop (which is the case
> > described in the commit message), but is there a way to handle the
> > post-loop devlink_alloc() failure case without printing the spurious
> > error message?
> 
> Sorry for not realising this when I made this suggestion in
> my review of v1.
> 
> Also, I would suggest splitting this patch in two: one patch
> per driver.

Hi Simon,

Apologies from my side as well; I overlooked the devlink_alloc failure 
path myself.

As noted, the loop index i is indeed invalid at that point. I will fix
this by decrementing i in the error path to ensure it points to the last
successfully initialized device before cleanup.

I will also split this patch into two separate patches in v3 as requested.

Regards,
Zilin

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

[Zilin Guan]

On Mon, Jan 26, 2026 at 03:52:52PM +0000, Simon Horman wrote:
> 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.)

Hi Simon,

Apologies for missing this dependency.

I will update the code to initialize lio->oct_dev alongside props->netdev
to prevent NULL pointer dereference in cleanup_rx_oom_poll_fn().

Thanks,
Zilin