Thread (28 messages) 28 messages, 2 authors, 11d ago

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help