Re: [PATCH v18 net-next 5/8] octeontx2: cn20k: Coordinate default rules with NIX LF lifecycle

[Ratheesh Kannoth]

On 2026-06-02 at 11:33:56, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) wrote:
> Add NIX_LF_DONT_FREE_DFT_IDXS so the PF can send NIX LF free during hw
> reinit or teardown without the AF freeing CN20K default NPC rule indexes
> while the driver still owns that state (otx2_init_hw_resources and
> otx2_free_hw_resources).
>
> On CN20K, allocate default NPC rules from NIX LF alloc before
> nix_interface_init, roll back with npc_cn20k_dft_rules_free on failure,
> and free from NIX LF free when the new flag is not set. Tighten
> rvu_mbox_handler_nix_lf_alloc error handling: use a single rc, propagate
> qmem_alloc and other errors, and set -ENOMEM only when kcalloc fails
> (remove the blanket -ENOMEM at the free_mem path).
>
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>

https://sashiko.dev/#/patchset/20260602060359.1894952-1-rkannoth%40marvell.com says

>>  	/* Alloc NIX RQ HW context memory and config the base */
>>  	hwctx_size = 1UL << ((ctx_cfg >> 4) & 0xF);
>> -	err = qmem_alloc(rvu->dev, &pfvf->rq_ctx, req->rq_cnt, hwctx_size);
>> -	if (err)
>> +	rc = qmem_alloc(rvu->dev, &pfvf->rq_ctx, req->rq_cnt, hwctx_size);
>> +	if (rc)
>>  		goto free_mem;
>>
>>  	pfvf->rq_bmap = kcalloc(req->rq_cnt, sizeof(long), GFP_KERNEL);
>> -	if (!pfvf->rq_bmap)
>> +	if (!pfvf->rq_bmap) {
>> +		rc = -ENOMEM;
>>  		goto free_mem;
>> +	}
>This is a pre-existing issue, but can repeated mailbox messages cause a memory
>leak here?
>If an untrusted VF continuously sends MBOX_MSG_NIX_LF_ALLOC messages without
>sending a corresponding NIX_LF_FREE, it appears qmem_alloc() and kcalloc()
>will overwrite the pointers in the pfvf structure (like pfvf->rq_ctx and
>pfvf->rq_bmap) without freeing the previously allocated memory.
>Could this be used by a guest to cause host DMA or heap memory exhaustion?
Prexisting issue, not addressing as part of this. May be a hardening series post this series,
or patch to introduce some limit  can be planned.

>> +	rc = nix_interface_init(rvu, pcifunc, intf, nixlf, rsp,
>> +				!!(req->flags & NIX_LF_LBK_BLK_SEL));
>> +	if (rc)
>> +		goto free_dft;
>>
>>  	/* Disable NPC entries as NIXLF's contexts are not initialized yet */
>>  	rvu_npc_disable_default_entries(rvu, pcifunc, nixlf);
>> @@ -1699,9 +1712,12 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu *rvu,
>>
>>  	goto exit;
>>
>> +free_dft:
>> +	if (is_cn20k(rvu->pdev))
>> +		npc_cn20k_dft_rules_free(rvu, pcifunc);
>Does this rollback path accidentally destroy pre-existing default rules?
>If the PF preserved the rules using NIX_LF_DONT_FREE_DFT_IDXS during teardown,
>npc_cn20k_dft_rules_alloc() returns 0 without allocating anything when they
>are already installed:
>npc_cn20k_dft_rules_alloc(struct rvu *rvu, u16 pcifunc)
>{
>    ...
>    ret = npc_cn20k_dft_rules_idx_get(rvu, pcifunc, &b, &m, &p, &u);
>    if (!ret) {
>        dev_dbg(rvu->dev, "default rules are already installed...");
>        return 0;
>    }
>    ...
>}
>If nix_interface_init() subsequently fails, the error handler jumps to free_dft
>and unconditionally frees the rules.
Even if it is freed, next successful nix lf alloc, will setup these default rules.

We can modify npc_cn20k_dft_rules_alloc() to add one more paremeter (bool &) to indicate whether newly/already
allocated status. Based on the flag, invoke npc_cn20k_dft_rules_free(). Will take it up as
hardening series after the series is merged.