Skip to contentSkip to conversation

octeontx2-af: fix CN20K counter bounds and mailbox base

RFC New Yicheng Zhong 1 participant 3 messages Nov 11

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover octeontx2-af: fix CN20K counter bounds and mailbox base

Two fixes in the CN20K paths of octeontx2-af.

The first bounds the counter argument of the MCAM counter handler before it is turned into a bank and a register offset. With a counter past the table the bank selects a bank that does not exist and the offset leaves the NPC block.

The second removes a phys_to_virt() from a pointer that is already a kernel virtual address, which otherwise takes probe out of the linear map on arm64.

Both were found by reading the driver and were exercised in a QEMU model of CN20K silicon, not on hardware, which is why they are RFC. A longer report covering other findings in the same handlers went to the maintainers and the security team separately and is not part of this series.

Yicheng Zhong (2):
octeontx2-af: reject an out of range MCAM counter on CN20K
octeontx2-af: cn20k: do not phys_to_virt() the mailbox base

 .../net/ethernet/marvell/octeontx2/af/cn20k/mbox_init.c    | 4 ++--
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c        | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

base-commit: 7b6ab4dbc156e2f96419715130b3d5738a306c93

Signature · 1 line
2.43.0
#1 lore raw

RFC 1/2 octeontx2-af: reject an out of range MCAM counter on CN20K

The CN20K branch of rvu_mbox_handler_npc_mcam_counter_stats() derives the bank and the register offset from the counter argument without checking it against the table, unlike the path the other silicons take. With a counter past mcam->total_entries the bank number selects a bank that does not exist, so the register offset leaves the NPC block and the caller reads a register of a neighbouring block.

Bound the counter to the table before deriving bank and index, which keeps the access inside the block; an ownership check would need the caller to be compared against the MCAM entry owner and is not added here.

Fixes 4e527f1e5c15 ("octeontx2-af: npc: cn20k: Add new mailboxes for CN20K silicon")
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +7 -0
index 42a601976db..be8e3518e7b 100644
@@ -4015,6 +4015,13 @@ int rvu_mbox_handler_npc_mcam_counter_stats(struct rvu *rvu,
 	 * has its own counter. No need to verify the counter index.
 	 */
 	if (is_cn20k(rvu->pdev)) {
+		/* cntr is an MCAM entry index on CN20K.  An index past the
+		 * table would derive a bank that does not exist and take the
+		 * register offset out of the NPC block.
+		 */
+		if (req->cntr >= mcam->total_entries)
+			return NPC_MCAM_INVALID_REQ;
+
 		index = req->cntr & (mcam->banksize - 1);
 		bank = npc_get_bank(mcam, req->cntr);
 		regval = rvu_read64(rvu, blkaddr,
Signature · 1 line
2.43.0
#2 lore raw

RFC 2/2 octeontx2-af: cn20k: do not phys_to_virt() the mailbox base

In reply to Yicheng Zhong

qmem_alloc() stores the kernel virtual address returned by otx2_dma_alloc_coherent() in ->base and the DMA address in ->iova, so ->base is already addressable. cn20k_rvu_get_mbox_regions() passes it through phys_to_virt() before handing it to otx2_mbox_regions_init(), which then dereferences the result as a pointer, and on arm64 phys_to_virt() subtracts PHYS_OFFSET and adds PAGE_OFFSET, so a virtual address goes in and something outside the linear map comes out.

Use ->base directly, since the memory the AF wants is the RAM it allocated. If ->base is meant to hold a physical address on this path then the assignment in qmem_alloc() is what should change instead, and we would like to know which of the two is intended.

Fixes f8909d3dd554 ("octeontx2-af: CN20K mbox implementation for AF's VF")
---
 drivers/net/ethernet/marvell/octeontx2/af/cn20k/mbox_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/marvell/octeontx2/af/cn20k/mbox_init.c +2 -2
index 01f32adac59..6908d7ff7fb 100644
@@ -240,7 +240,7 @@ int cn20k_rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
 			if (!test_bit(region, pf_bmap))
 				continue;
 
-			bar = (u64)phys_to_virt((u64)rvu->ng_rvu->vf_mbox_addr->base);
+			bar = (u64)rvu->ng_rvu->vf_mbox_addr->base;
 			bar += region * MBOX_SIZE;
 			mbox_addr[region] = (void *)bar;
 
@@ -254,7 +254,7 @@ int cn20k_rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
 		if (!test_bit(region, pf_bmap))
 			continue;
 
-		bar = (u64)phys_to_virt((u64)rvu->ng_rvu->pf_mbox_addr->base);
+		bar = (u64)rvu->ng_rvu->pf_mbox_addr->base;
 		bar += region * MBOX_SIZE;
 
 		mbox_addr[region] = (void *)bar;
Signature · 1 line
2.43.0