[PATCH net] octeontx2-af: npc: Fix size of entry2cntr_map

[PATCH net] octeontx2-af: npc: Fix size of entry2cntr_map

[Ratheesh Kannoth]

octeontx2-af: Fix out of bound access in entry2counter array.
KASAN prints below splat. This is caused by allocating counter for
reserved mcam entry for cpt 2nd pass entry. But mcam->entry2cntr_map
is not allocated for reserved entries.

 ==================================================================
 BUG: KASAN: slab-out-of-bounds in npc_map_mcam_entry_and_cntr+0xb0/0x1a0
 Write of size 2 at addr ffff0001033e7ffe by task kworker/0:1/14

 CPU: 0 PID: 14 Comm: kworker/0:1 Not tainted 6.1.67 #1
 Hardware name: Marvell CN106XX board (DT)
 Workqueue: events work_for_cpu_fn
 Call trace:
  dump_backtrace.part.0+0xe4/0xf0
  show_stack+0x18/0x30
  dump_stack_lvl+0x88/0xb4
  print_report+0x154/0x458
  kasan_report+0xb8/0x194
  __asan_store2+0x7c/0xa0
  npc_map_mcam_entry_and_cntr+0xb0/0x1a0
  rvu_mbox_handler_npc_mcam_write_entry+0x268/0x280
  npc_install_flow+0x840/0xfe0
  rvu_npc_install_cpt_pass2_entry+0x138/0x190
  rvu_nix_init+0x148c/0x2880
  rvu_probe+0x1800/0x30b0
  local_pci_probe+0x78/0xe0
  work_for_cpu_fn+0x30/0x50
  process_one_work+0x4cc/0x97c
  worker_thread+0x360/0x630
  kthread+0x1a0/0x1b0
  ret_from_fork+0x10/0x20

Fixes: 55307fcb9258 ("octeontx2-af: Add mbox messages to install and delete MCAM rules")
Cc: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
index d301a3f0f87a..12f525b5df7b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
@@ -2181,7 +2181,7 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
 	/* Alloc memory for MCAM entry to counter mapping and for tracking
 	 * counter's reference count.
 	 */
-	mcam->entry2cntr_map = kcalloc(mcam->bmap_entries, sizeof(u16),
+	mcam->entry2cntr_map = kcalloc(mcam->total_entries, sizeof(u16),
 				       GFP_KERNEL);
 	if (!mcam->entry2cntr_map)
 		goto free_cntr_map;
@@ -2197,10 +2197,11 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
 	if (!mcam->entry2target_pffunc)
 		goto free_cntr_refcnt;
 
-	for (index = 0; index < mcam->bmap_entries; index++) {
+	for (index = 0; index < mcam->bmap_entries; index++)
 		mcam->entry2pfvf_map[index] = NPC_MCAM_INVALID_MAP;
+
+	for (index = 0; index < mcam->total_entries; index++)
 		mcam->entry2cntr_map[index] = NPC_MCAM_INVALID_MAP;
-	}
 
 	for (cntr = 0; cntr < mcam->counters.max; cntr++)
 		mcam->cntr2pfvf_map[cntr] = NPC_MCAM_INVALID_MAP;
-- 
2.43.0

Re: [PATCH net] octeontx2-af: npc: Fix size of entry2cntr_map

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
octeontx2-af: npc: Fix size of entry2cntr_map

octeontx2-af: Fix out of bound access in entry2counter array.
KASAN prints below splat. This is caused by allocating counter for
reserved mcam entry for cpt 2nd pass entry. But mcam->entry2cntr_map
is not allocated for reserved entries.

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index d301a3f0f87a8..12f525b5df7bb 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -2181,7 +2181,7 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
>  	/* Alloc memory for MCAM entry to counter mapping and for tracking
>  	 * counter's reference count.
>  	 */
> -	mcam->entry2cntr_map = kcalloc(mcam->bmap_entries, sizeof(u16),
> +	mcam->entry2cntr_map = kcalloc(mcam->total_entries, sizeof(u16),
>  				       GFP_KERNEL);

[Severity: High]
Now that entry2cntr_map is allocated to total_entries to allow counters on
reserved entries, do the cleanup loops also need to be updated?

Looking at __npc_mcam_free_counter() and
rvu_mbox_handler_npc_mcam_unmap_counter(), they appear to only iterate up to
mcam->bmap_entries and search the unreserved bitmap:

__npc_mcam_free_counter() {
        ...
        while (entry < mcam->bmap_entries) {
                if (!mcam->cntr_refcnt[req->cntr])
                        break;
                index = find_next_bit(mcam->bmap, mcam->bmap_entries, entry);
        ...
}

Does this skip unmapping counters that are attached to reserved MCAM entries?

If so, could the hardware STAT_ACT register remain active and leak the
counter's reference count, causing cross-tenant statistics corruption when
the counter is later reallocated?

>  	if (!mcam->entry2cntr_map)
>  		goto free_cntr_map;

-- 
pw-bot: cr