netdev - [bug report] net/funeth: probing and netdev ops

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

Message-ID: <ec0b7b3e-0d69-4fa7-bfd2-b3b110fe237d@stanley.mountain>
Date: Tue, 11 Mar 2025 18:01:33 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Dimitris Michailidis <d.michailidis@...gible.com>
Cc: netdev@...r.kernel.org
Subject: [bug report] net/funeth: probing and netdev ops

Hello Dimitris Michailidis,

Commit ee6373ddf3a9 ("net/funeth: probing and netdev ops") from Feb
24, 2022 (linux-next), leads to the following Smatch static checker
warning:

	drivers/net/ethernet/fungible/funeth/funeth_main.c:333 fun_alloc_queue_irqs()
	warn: 'irq' can also be NULL

drivers/net/ethernet/fungible/funeth/funeth_main.c
    319 static int fun_alloc_queue_irqs(struct net_device *dev, unsigned int ntx,
    320                                 unsigned int nrx)
    321 {
    322         struct funeth_priv *fp = netdev_priv(dev);
    323         int node = dev_to_node(&fp->pdev->dev);
    324         struct fun_irq *irq;
    325         unsigned int i;
    326 
    327         for (i = fp->num_tx_irqs; i < ntx; i++) {
    328                 irq = fun_alloc_qirq(fp, i, node, 0);
                               ^^^^^^^^^^^^^
The fun_alloc_qirq() function can return NULL.

    329                 if (IS_ERR(irq))
    330                         return PTR_ERR(irq);
    331 
    332                 fp->num_tx_irqs++;
--> 333                 netif_napi_add_tx(dev, &irq->napi, fun_txq_napi_poll);
    334         }
    335 

The problem is this:

   249  static struct fun_irq *fun_alloc_qirq(struct funeth_priv *fp, unsigned int idx,
   250                                        int node, unsigned int xa_idx_offset)
   251  {
   252          struct fun_irq *irq;
   253          int cpu, res;
   254  
   255          cpu = cpumask_local_spread(idx, node);
   256          node = cpu_to_mem(cpu);
   257  
   258          irq = kzalloc_node(sizeof(*irq), GFP_KERNEL, node);
   259          if (!irq)
   260                  return ERR_PTR(-ENOMEM);
   261  
   262          res = fun_reserve_irqs(fp->fdev, 1, &irq->irq_idx);
   263          if (res != 1)
   264                  goto free_irq;

The error code is not set on this path.  This is the only caller.  Why not
modify fun_reserve_irqs() to just return zero on success and negative
failures?  Are we likely to need the current API in the near future?

   265  
   266          res = xa_insert(&fp->irqs, idx + xa_idx_offset, irq, GFP_KERNEL);
   267          if (res)
   268                  goto release_irq;
   269  
   270          irq->irq = pci_irq_vector(fp->pdev, irq->irq_idx);
   271          cpumask_set_cpu(cpu, &irq->affinity_mask);
   272          irq->aff_notify.notify = fun_irq_aff_notify;
   273          irq->aff_notify.release = fun_irq_aff_release;
   274          irq->state = FUN_IRQ_INIT;
   275          return irq;
   276  
   277  release_irq:
   278          fun_release_irqs(fp->fdev, 1, &irq->irq_idx);
   279  free_irq:
   280          kfree(irq);
   281          return ERR_PTR(res);
   282  }

regards,
dan carpenter