Re: [PATCH v19 net-next 9/9] octeontx2-af: npc: cn20k: Allocate npc_priv and dstats dynamically.
From: Ratheesh Kannoth <hidden>
Date: 2026-06-08 02:25:12
Also in:
lkml
On 2026-06-05 at 12:02:45, Ratheesh Kannoth (rkannoth@marvell.com) wrote:
Replace the file-scope static npc_priv with a kcalloc'd struct filled from hardware bank/subbank geometry at init (num_banks is no longer a const compile-time constant; drop init_done and use a non-NULL npc_priv pointer for liveness). Thread npc_priv_get() / pointer access through the CN20K NPC code paths, extend teardown to kfree the root struct on failure and in npc_cn20k_deinit, and adjust MCAM section setup to use the discovered subbank count. Allocate MCAM debugfs dstats via devm_kzalloc instead of a static matrix, and use the allocated backing store consistently when computing deltas (including the counter rollover compare). Signed-off-by: Ratheesh Kannoth <redacted>
https://sashiko.dev/#/patchset/20260605063245.3553861-1-rkannoth%40marvell.com says
quoted
*/ -static u64 dstats[MAX_NUM_BANKS][MAX_SUBBANK_DEPTH * MAX_NUM_SUB_BANKS] = {}; +static u64 (*dstats)[MAX_NUM_BANKS][MAX_SUBBANK_DEPTH * MAX_NUM_SUB_BANKS]; + static int npc_mcam_dstats_show(struct seq_file *s, void *unused) {What happens if npc_priv is NULL when this function is called?
npc_priv is allocated during probe() much before debugfs entries are created.
quoted
debugfs_create_file("vidx2idx", 0444, rvu->rvu_dbg.npc, npc_priv, &npc_vidx2idx_map_fops); + dstats = devm_kzalloc(rvu->dev, sizeof(*dstats), GFP_KERNEL); + if (!dstats) + return -ENOMEM; +Does allocating dstats per-device using devm_kzalloc overwrite the global dstats pointer in multi-device systems?
Only one AF device per system, first patch in the series enforce the same.
quoted
int bank_off, sb_id; /* mcam_idx should be less than (2 * bank depth) */ - if (mcam_idx >= npc_priv.bank_depth * 2) { + if (mcam_idx >= npc_priv->bank_depth * 2) {Is it possible for unprivileged Virtual Functions to trigger a denial of service here? Mailbox handlers invoke npc_mcam_idx_2_key_type(). If a VF sends a mailbox message early during host probe, or after an NPC initialization failure, npc_priv might be NULL. Would this crash the host kernel without a NULL pointer check?
Not possibe; as VF/PF can be probed only after AF is initialized properly.
quoted
- rsp->kw_type = npc_priv.kw; - rsp->x4_slots = npc_priv.bank_depth; + rsp->kw_type = npc_priv->kw; + rsp->x4_slots = npc_priv->bank_depth; return 0; }Could this also crash if npc_priv is NULL? Since mailbox handlers process messages from Virtual Functions, checking npc_priv for NULL before dereferencing seems necessary to prevent a crash.
Not possibe; as VF/PF can be probed only after AF is initialized properly.
- npc_priv.num_subbanks = num_subbanks;quoted
- npc_priv.bank_depth = bank_depth; - npc_priv.subbank_depth = subbank_depth; + npc_priv = kcalloc(1, sizeof(*npc_priv), GFP_KERNEL);Will probing a second RVU device overwrite the npc_priv pointer allocated by the first device? Since npc_priv remains a global static pointer, it appears this should be stored in a per-device structure like struct rvu rather than a global variable to support multi-device setups safely.
Only one AF device per system, first patch in the series enforce the same.
quoted
+ kfree(npc_priv); + npc_priv = NULL; }If any single RVU device unbinds or fails probing, will this free the global npc_priv and set it to NULL? If so, this seems like it would corrupt the shared state and cause use-after-free or NULL pointer crashes for other active RVU devices on the system.
Ubinding and failuer handling in AF driver is not complete; as is not in the scope of this patch. Will work on a proper error handlling (hardening patch series) once this series is merged.