Re: [PATCH v14 net-next 7/9] octeontx2-af: npc: Support for custom KPU profile from filesystem

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-05-14 at 11:55:35, Ratheesh Kannoth (rkannoth@marvell.com) wrote:
> Flashing updated firmware on deployed devices is cumbersome. Provide a
> mechanism to load a custom KPU (Key Parse Unit) profile directly from
> the filesystem at module load time.
>
> When the rvu_af module is loaded with the kpu_profile parameter, the
> specified profile is read from /lib/firmware/kpu and programmed into
> the KPU registers. Add npc_kpu_profile_cam2 for the extended cam format
> used by filesystem-loaded profiles and support ptype/ptype_mask in
> npc_config_kpucam when profile->from_fs is set.
>

>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
>> index ec5b2d6482462..21eb15f38c009 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
>[ ... ]
>> @@ -1788,26 +1872,175 @@ static void npc_prepare_default_kpu(struct rvu *rvu,
>>  	npc_cn20k_update_action_entries_n_flags(rvu, profile);
>>  }
>>
>> +static int npc_alloc_kpu_cam2_n_action2(struct rvu *rvu, int kpu_num,
>> +					int num_entries)
>> +{
>> +	struct npc_kpu_profile_adapter *adapter = &rvu->kpu;
>> +	struct npc_kpu_profile *kpu;
>> +
>> +	kpu = &adapter->kpu[kpu_num];
>> +
>> +	kpu->cam2 = devm_kcalloc(rvu->dev, num_entries,
>> +				 sizeof(*kpu->cam2), GFP_KERNEL);
>Since adapter->kpu can point to the global array npc_kpu_profiles
>(assigned in npc_prepare_default_kpu), does this store a device-managed
>allocation inside a global structure?
>If the device unbinds, the devm subsystem will automatically free this
>memory. Might this leave dangling pointers in the global state, leading to a
>use-after-free regression for concurrent or subsequent RVU device probes?

The concern regarding devm-allocated memory pointers persisting in a
global structure is valid in multi-instance architectures, but it is
not operational in this system due to hardware and driver life-cycle
guarantees.

First, the system topology restricts the platform to exactly one physical
RVU Admin Function (AF) device instance across the entire lifetime of the
running system. Because there are never concurrent or parallel RVU AF
hardware device instances probing the system, cross-device global pointer
corruption or concurrent use-after-free conditions cannot occur.

Second, the life cycle of the global profile structure is tied directly
to the lifetime of the underlying AF driver module. When the AF driver is
unbound or unloaded, the module's exit pathways discard the subsystem
state entirely. Since no subsequent or concurrent probe sequence can run
independently of this shared lifecycle, the devm-freed pointers inside the
adapter layout never present a true runtime dangling hazard.

>> +	if (!kpu->cam2)
>> +		return -ENOMEM;
>> +
>> +	kpu->action2 = devm_kcalloc(rvu->dev, num_entries,
>> +				    sizeof(*kpu->action2), GFP_KERNEL);
>> +	if (!kpu->action2)
>> +		return -ENOMEM;
>> +
>> +	return 0;
>> +}
>> +
>> +static int npc_apply_custom_kpu_from_fw(struct rvu *rvu,
>> +					struct npc_kpu_profile_adapter *profile)
>> +{
>> +	size_t hdr_sz = sizeof(struct npc_kpu_profile_fwdata), offset = 0;
>> +	const struct npc_kpu_profile_fwdata *fw;
>> +	struct npc_kpu_profile_action *action;
>> +	struct npc_kpu_profile_cam *cam;
>> +	struct npc_kpu_fwdata *fw_kpu;
>> +	int entries, entry, kpu;
>> +
>> +	fw = rvu->kpu_fwdata;
>> +
>> +	for (kpu = 0; kpu < fw->kpus; kpu++) {
>> +		if (rvu->kpu_fwdata_sz < hdr_sz + offset) {
>Is this check sufficient to read the fw_kpu structure safely? The condition
>only verifies that the starting offset is within bounds, but immediately
>below we dereference fw_kpu->entries.
>If the remaining firmware data is less than sizeof(*fw_kpu), it seems this
>will result in an out-of-bounds read. The new npc_apply_custom_kpu_from_fs
>function handles this correctly by checking for
>hdr_sz + offset + sizeof(*fw_kpu).

The observation regarding the potential out-of-bounds boundary check
is correct, but it targets legacy parsing logic that is completely
outside the functional scope of this specific patch series.

The npc_apply_custom_kpu_from_fw() is existing code that was
merely refactored and moved to accommodate upstream review comments.
It does not introduce the sub-optimal length check as a regression.

This length validation issue will be isolated and submitted
independently as a standalone bug fix patch targeting the net tree.

>> +static int npc_apply_custom_kpu_from_fs(struct rvu *rvu,
>> +					struct npc_kpu_profile_adapter *profile)
>> +{
>> +	size_t hdr_sz = sizeof(struct npc_kpu_profile_fwdata), offset = 0;
>> +	const struct npc_kpu_profile_fwdata *fw;
>> +	struct npc_kpu_profile_action *action;
>> +	struct npc_kpu_profile_cam2 *cam2;
>> +	struct npc_kpu_fwdata *fw_kpu;
>> +	int entries, ret, entry, kpu;
>> +
>> +	fw = rvu->kpu_fwdata;
>> +
>> +	/* Binary blob contains ikpu actions entries at start of data[0] */
>> +	profile->ikpu2 = devm_kcalloc(rvu->dev, 1,
>> +				      sizeof(ikpu_action_entries),
>> +				      GFP_KERNEL);
>> +	if (!profile->ikpu2)
>> +		return -ENOMEM;
>> +
>> +	action = (struct npc_kpu_profile_action *)(fw->data + offset);
>> +
>> +	if (rvu->kpu_fwdata_sz < hdr_sz + sizeof(ikpu_action_entries))
>> +		return -EINVAL;
>Could this lead to a memory leak of profile->ikpu2 and other devm_kcalloc
>allocations if the firmware parsing fails midway?
>Since npc_load_kpu_profile catches the error and falls back to the default
>profile, the device probe ultimately succeeds. The devm allocations from the
>failed filesystem attempt appear to remain stranded for the entire lifetime
>of the device.

The concern regarding the lifetime of the devm allocations during a
fallback scenario is understandable, but it does not represent a leaking
hazard or an operational bug.

Because these allocations are bound to the device's lifecycle using the
managed device resource framework (devm_kcalloc), the kernel guarantees
automatic reclamation when the underlying RVU physical device unbinds
or removes.

While the failed filesystem parsing attempt results in a fallback to
the default profile, the device probe itself still completes successfully.
The allocated profile block remains mapped to the operational device instance
as an unused buffer structure, rather than an untracked, stranded memory
leak. Explicitly rolling back or manually freeing devm memory within the
probe failure path would duplicate the clean-up routines inherently managed
by the devm subsystem, adding redundant code paths for a non-breaking,
one-time configuration fallback.