netdev - [QUERY] net: octeontx2: query on mutex_unlock() usage and WRITE

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: <20250628165509.3976081-1-alok.a.tiwari@oracle.com>
Date: Sat, 28 Jun 2025 09:55:06 -0700
From: Alok Tiwari <alok.a.tiwari@...cle.com>
To: sgoutham@...vell.com, gakula@...vell.com, sbhatta@...vell.com,
        hkelam@...vell.com, bbhushan2@...vell.com, andrew+netdev@...n.ch,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, netdev@...r.kernel.org
Cc: alok.a.tiwari@...cle.com
Subject: [QUERY] net: octeontx2: query on mutex_unlock() usage and WRITE_ONCE omission

Noticed a couple of points in rep.c that might need attention:

1.
Use of mutex_unlock(&priv->mbox.lock) in rvu_rep_mcam_flow_init()

mutex_unlock(&priv->mbox.lock);
This function does not explicitly acquire mbox.lock, yet it calls mutex_unlock().
Could you confirm whether this is a bug or if the lock is acquired implicitly in a helper function like otx2_sync_mbox_msg()
or a possible leftover?

2.
Direct assignment of dev->mtu without WRITE_ONCE in rvu_rep_change_mtu()

dev->mtu = new_mtu;
Should this use WRITE_ONCE() to ensure safe access in concurrent scenarios?

Thanks,
Alok
---
 drivers/net/ethernet/marvell/octeontx2/nic/rep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
index 2cd3da3b6843..88dddf1fffb3 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
@@ -88,7 +88,7 @@ static int rvu_rep_mcam_flow_init(struct rep_dev *rep)
 		sort(&rep->flow_cfg->flow_ent[0], allocated,
 		     sizeof(rep->flow_cfg->flow_ent[0]), mcam_entry_cmp, NULL);
 
-	mutex_unlock(&priv->mbox.lock);
+	mutex_unlock(&priv->mbox.lock); // why mutex_unlock here 
 
 	rep->flow_cfg->max_flows = allocated;
 
@@ -323,7 +323,7 @@ static int rvu_rep_change_mtu(struct net_device *dev, int new_mtu)
 
 	netdev_info(dev, "Changing MTU from %d to %d\n",
 		    dev->mtu, new_mtu);
-	dev->mtu = new_mtu;
+	dev->mtu = new_mtu; // < Is there a reason WRITE_ONCE isn't used here
 
 	evt.evt_data.mtu = new_mtu;
 	evt.pcifunc = rep->pcifunc;
-- 
2.47.1