[PATCH net v3] octeontx2-af: Fix BPID leak in nix_bp_enable()

[PATCH net v3] octeontx2-af: Fix BPID leak in nix_bp_enable()

[nshettyj]

From: Rakesh Kudurumalla <rkudurumalla@marvell.com>

For LBK interfaces, rvu_nix_get_bpid() allocates a BPID from the free
pool on every call. nix_bp_enable() called it unconditionally before
the loop, and again after the last channel was programmed, leaking a
BPID whenever that extra call's result went unused. With
req->chan_cnt == 0, the pre-loop call leaked a BPID on every call.

Move the allocation into the loop body so it runs exactly once per
channel actually programmed, and reject req->chan_cnt == 0 upfront.

Fixes: d6212d2e41a0 ("octeontx2-af: Create BPIDs free pool")
Signed-off-by: Nitin Shetty J <nshettyj@marvell.com>
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
changes in v3:
- Unwind BPID allocations and disable programmed channels on mid-loop failure in `nix_bp_enable()`.
- Report the actual BPID written per channel instead of reconstructing it arithmetically.
- Validate the BPID range in `nix_bp_disable()` before freeing it, preventing an out-of-bounds write.

changes in v2:
- Move the rvu_nix_get_bpid() call for LBK BPID allocation from before
  the loop into the loop body.
- Validate req->chan_cnt before allocating BPIDs.
- updated commit message and fix tag
---
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   | 79 ++++++++++++++++---
 1 file changed, 66 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 153eb57bad06..1ef505009c3a 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -631,6 +631,15 @@ static int nix_bp_disable(struct rvu *rvu,
 
 		if (type == NIX_INTF_TYPE_LBK) {
 			bpid = cfg & GENMASK(8, 0);
+			/* Ignore channels that were never armed with an LBK
+			 * free-pool bpid (e.g. never enabled, or belonging
+			 * to a CGX/SDP range) - bpid - free_pool_base would
+			 * underflow and corrupt an unrelated bitmap word.
+			 */
+			if (bpid < bp->free_pool_base ||
+			    bpid >= bp->free_pool_base + bp->bpids.max)
+				continue;
+
 			mutex_lock(&rvu->rsrc_lock);
 			rvu_free_rsrc(&bp->bpids, bpid - bp->free_pool_base);
 			for (bpid = 0; bpid < bp->bpids.max; bpid++) {
@@ -738,6 +747,35 @@ static int rvu_nix_get_bpid(struct rvu *rvu, struct nix_bp_cfg_req *req,
 	return bpid;
 }
 
+static void nix_bp_enable_unwind(struct rvu *rvu, struct nix_bp *bp,
+				 int blkaddr, u16 chan_base, int chan_cnt,
+				 int type, bool cpt_link)
+{
+	u16 chan, chan_v, bpid;
+	u64 cfg;
+
+	for (chan = chan_base; chan < chan_base + chan_cnt; chan++) {
+		chan_v = nix_get_channel(chan, cpt_link);
+		cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v));
+		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
+			    cfg & ~BIT_ULL(16));
+
+		if (type != NIX_INTF_TYPE_LBK)
+			continue;
+
+		bpid = cfg & GENMASK_ULL(8, 0);
+		if (bpid < bp->free_pool_base ||
+		    bpid >= bp->free_pool_base + bp->bpids.max)
+			continue;
+
+		mutex_lock(&rvu->rsrc_lock);
+		rvu_free_rsrc(&bp->bpids, bpid - bp->free_pool_base);
+		bp->fn_map[bpid - bp->free_pool_base] = 0;
+		bp->ref_cnt[bpid - bp->free_pool_base] = 0;
+		mutex_unlock(&rvu->rsrc_lock);
+	}
+}
+
 static int nix_bp_enable(struct rvu *rvu,
 			 struct nix_bp_cfg_req *req,
 			 struct nix_bp_cfg_rsp *rsp,
@@ -747,9 +785,12 @@ static int nix_bp_enable(struct rvu *rvu,
 	u16 pcifunc = req->hdr.pcifunc;
 	struct rvu_pfvf *pfvf;
 	u16 chan_base, chan;
-	s16 bpid, bpid_base;
+	struct nix_hw *nix_hw;
+	struct nix_bp *bp;
 	u16 chan_v;
+	s16 bpid;
 	u64 cfg;
+	int err;
 
 	pf = rvu_get_pf(rvu->pdev, pcifunc);
 	type = is_lbk_vf(rvu, pcifunc) ? NIX_INTF_TYPE_LBK : NIX_INTF_TYPE_CGX;
@@ -764,16 +805,27 @@ static int nix_bp_enable(struct rvu *rvu,
 	if (cpt_link && !rvu->hw->cpt_links)
 		return 0;
 
+	if (!req->chan_cnt)
+		return NIX_AF_ERR_INVALID_BPID_REQ;
+
 	pfvf = rvu_get_pfvf(rvu, pcifunc);
-	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
+	err = nix_get_struct_ptrs(rvu, pcifunc, &nix_hw, &blkaddr);
+	if (err)
+		return err;
 
-	bpid_base = rvu_nix_get_bpid(rvu, req, type, chan_id);
+	bp = &nix_hw->bp;
 	chan_base = pfvf->rx_chan_base + req->chan_base;
-	bpid = bpid_base;
 
 	for (chan = chan_base; chan < (chan_base + req->chan_cnt); chan++) {
+		bpid = rvu_nix_get_bpid(rvu, req, type, chan_id);
 		if (bpid < 0) {
 			dev_warn(rvu->dev, "Fail to enable backpressure\n");
+			/* Undo the channels already enabled/allocated for
+			 * this request so their BPIDs and armed channels
+			 * don't leak until an FLR.
+			 */
+			nix_bp_enable_unwind(rvu, bp, blkaddr, chan_base,
+					     chan_id, type, cpt_link);
 			return -EINVAL;
 		}
 
@@ -783,16 +835,17 @@ static int nix_bp_enable(struct rvu *rvu,
 		cfg &= ~GENMASK_ULL(8, 0);
 		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
 			    cfg | (bpid & GENMASK_ULL(8, 0)) | BIT_ULL(16));
-		chan_id++;
-		bpid = rvu_nix_get_bpid(rvu, req, type, chan_id);
-	}
 
-	for (chan = 0; chan < req->chan_cnt; chan++) {
-		/* Map channel and bpid assign to it */
-		rsp->chan_bpid[chan] = ((req->chan_base + chan) & 0x7F) << 10 |
-					(bpid_base & 0x3FF);
-		if (req->bpid_per_chan)
-			bpid_base++;
+		/* Report the bpid actually programmed for this channel,
+		 * instead of reconstructing it arithmetically from the
+		 * first channel's bpid. For LBK, each call above can
+		 * return a non-contiguous bpid from the shared free pool,
+		 * so that reconstruction can diverge from what's actually
+		 * written into NIX_AF_RX_CHANX_CFG.
+		 */
+		rsp->chan_bpid[chan_id] = ((req->chan_base + chan_id) & 0x7F) << 10 |
+					  (bpid & 0x3FF);
+		chan_id++;
 	}
 	rsp->chan_cnt = req->chan_cnt;
 
-- 
2.48.1

Re: [PATCH net v3] octeontx2-af: Fix BPID leak in nix_bp_enable()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.