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

Message-ID: <f9fa829d-2580-4b49-b0c6-cf2e2a8f6cac@stanley.mountain>
Date: Thu, 15 Aug 2024 14:29:38 +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:475 fun_free_rings()
	warn: 'rxqs' was already freed. (line 472)

drivers/net/ethernet/fungible/funeth/funeth_main.c
    441 static void fun_free_rings(struct net_device *netdev, struct fun_qset *qset)
    442 {
    443         struct funeth_priv *fp = netdev_priv(netdev);
    444         struct funeth_txq **xdpqs = qset->xdpqs;
    445         struct funeth_rxq **rxqs = qset->rxqs;
    446 
    447         /* qset may not specify any queues to operate on. In that case the
    448          * currently installed queues are implied.
    449          */
    450         if (!rxqs) {
    451                 rxqs = rtnl_dereference(fp->rxqs);
    452                 xdpqs = rtnl_dereference(fp->xdpqs);
    453                 qset->txqs = fp->txqs;
    454                 qset->nrxqs = netdev->real_num_rx_queues;
    455                 qset->ntxqs = netdev->real_num_tx_queues;
    456                 qset->nxdpqs = fp->num_xdpqs;
    457         }
    458         if (!rxqs)
    459                 return;
    460 
    461         if (rxqs == rtnl_dereference(fp->rxqs)) {
    462                 rcu_assign_pointer(fp->rxqs, NULL);
    463                 rcu_assign_pointer(fp->xdpqs, NULL);
    464                 synchronize_net();
    465                 fp->txqs = NULL;
    466         }
    467 
    468         free_rxqs(rxqs, qset->nrxqs, qset->rxq_start, qset->state);
    469         free_txqs(qset->txqs, qset->ntxqs, qset->txq_start, qset->state);
    470         free_xdpqs(xdpqs, qset->nxdpqs, qset->xdpq_start, qset->state);
    471         if (qset->state == FUN_QSTATE_DESTROYED)
    472                 kfree(rxqs);
                        ^^^^^^^^^^^
Freed.

    473 
    474         /* Tell the caller which queues were operated on. */
--> 475         qset->rxqs = rxqs;
                             ^^^^^
why are we saving a freed pointer?

    476         qset->xdpqs = xdpqs;
    477 }

regards,
dan carpenter