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
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
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.)
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
© 2016 - 2026 Red Hat, Inc.