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

[Ratheesh Kannoth]

On 2026-06-05 at 12:02:43, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) 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.
>
> Usage:
>   1. Copy the KPU profile file to /lib/firmware/kpu.
>   2. Build OCTEONTX2_AF as a module.
>   3. Load: insmod rvu_af.ko kpu_profile=<profile_name>
>
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>

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

>>
>>  	/* Verify if profile fits the HW */
>> +	if (fw->kpus > rvu->hw->npc_kpus) {
>> +		dev_warn(rvu->dev, "Not enough KPUs: %d > %d\n", fw->kpus,
>> +			 rvu->hw->npc_kpus);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Check if there is enough memory */
>>  	if (fw->kpus > profile->kpus) {
>This is a pre-existing issue, but does this code read out-of-bounds when
>parsing firmware KPU profiles?
Pre-existing issue, will address as a seperate patch after this series
is merged.

>>
>> +	for (kpu = 0; kpu < fw->kpus; kpu++) {
>> +		if (rvu->kpu_fwdata_sz < hdr_sz + offset) {
>> +			dev_warn(rvu->dev,
>> +				 "Profile size mismatch on KPU%i parsing\n",
>> +				 kpu + 1);
>> +			return -EINVAL;
>> +		}
>> +
>> +		fw_kpu = (struct npc_kpu_fwdata *)(fw->data + offset);
>> +		if (fw_kpu->entries < 0) {
>This is also a pre-existing issue, but does this check properly prevent an
>out-of-bounds read?
Pre-existing issue, will address as a seperate patch after this series
is merged.

>> +	/* Binary blob contains ikpu actions entries at start of data[0] */
>> +	profile->ikpu2 = devm_kcalloc(rvu->dev, 1,
>> +				      sizeof(ikpu_action_entries),
>> +				      GFP_KERNEL);
>Will this leak devm-managed memory if filesystem firmware loading fails?
Yes. Buti it not an issue as there is only one AF device per system. 1st patch in the series
do enforce the same.

> +	/* The firmware layout does dependent on the internal size of
>> +	 * ikpu_action_entries.
>> +	 */
>> +	memcpy((void *)profile->ikpu2, action, sizeof(ikpu_action_entries));
>> +	offset += sizeof(ikpu_action_entries);
>Does this tightly couple the firmware ABI to the kernel's compile-time
>array size?
Yes. There is no field in the structure to indicate the length. We cant add
now as it will break backward compatability, so we maintain same entries all
time.

>The filesystem-based KPU profile parser copies data and advances its buffer
>offset using the compile-time sizeof(ikpu_action_entries). If future kernels
>add new packet kinds (PKINDs), the array size will increase.
>Because the firmware binary does not encode the length of this section, older
>firmware loaded onto a newer kernel will be parsed incorrectly, shifting the
>offset and causing the driver to read garbage for the remaining KPU headers,
>which breaks firmware ABI compatibility.