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

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-05-20 at 07:39:37, 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.
>

>> -static int npc_apply_custom_kpu(struct rvu *rvu,
>> -				struct npc_kpu_profile_adapter *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 npc_prepare_default_kpu() sets adapter->kpu to point to the global
>static array npc_kpu_profiles, does this allocate device-managed memory
>and store it in a global structure?
>If a custom filesystem profile is loaded, devm_kcalloc() will associate the
>allocation with the current rvu->dev. When the device is unbound, the memory
>is freed but the global npc_kpu_profiles array retains dangling pointers.
>Could this lead to a use-after-free or cross-device corruption if multiple
>RVU devices are probed?

The RVU Admin Function (AF) is a singleton
hardware block within the SoC architecture. There is structurally only a
single instance of this device probed per system, precluding any risk of cross-
device pointer corruption or multi-instance allocation conflicts.

Since there is a 1:1 mapping between the rvu->dev instance
and the global hardware profile structures, the lifespan of the devm-allocated
memory is structurally bound to the lifespan of the entire driver module. The
pointers inside the global array are only populated upon a successful probe
of this unique instance and are discarded if the block is unbound, preventing
nominal use-after-free conditions.

>This is a pre-existing issue, but modifying kpu->cam2 and kpu->action2, or
>copying custom entries directly into kpu->cam and kpu->action, mutates the
>default global npc_kpu_profiles array in-place. Does this inadvertently
>affect other RVU devices that might be relying on the default profile?

The driver operates under distinct state
machines for profile initialization. If a custom profile is successfully
loaded from  filesystem, the driver diverts from and entirely
bypasses the default global 'npc_kpu_profiles' array path.

n-place mutations to 'kpu->cam2' and
'kpu->action2' occur strictly within the context of a dynamically allocated
custom adapter profile. Because custom profiles instantiate independent memory
buffers.

> +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_fwdata *fw;
>>  	struct npc_kpu_profile_cam *cam;
>>  	struct npc_kpu_fwdata *fw_kpu;
>> -	int entries;
>> -	u16 kpu, entry;
>> +	int entries, entry, kpu;
>>
>> -	if (is_cn20k(rvu->pdev))
>> -		return npc_cn20k_apply_custom_kpu(rvu, profile);
>> +	fw = rvu->kpu_fwdata;
>> +
>> +	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 a pre-existing issue, but is the bounds check here sufficient to
>prevent out-of-bounds memory accesses?
>The check validates that hdr_sz + offset fits within rvu->kpu_fwdata_sz,
>but it doesn't include the size of the fw_kpu structure itself.
>If the remaining firmware buffer exactly equals offset, wouldn't reading
>fw_kpu->entries access memory past the end of the firmware buffer?

This is a legacy code issue and will fix as part of a follow up patch to net tree.

>> +				struct npc_kpu_profile_adapter *profile,
>> +				bool from_fs, int *fw_kpus)
>> +{
>> +	size_t hdr_sz = sizeof(struct npc_kpu_profile_fwdata);
>> +	const struct npc_kpu_profile_fwdata *fw;
>> +	struct npc_kpu_profile_fwdata *sfw;
>[ ... ]
>> +	*fw_kpus = fw->kpus;
>> +
>> +	sfw = devm_kcalloc(rvu->dev, 1, sizeof(*sfw), GFP_KERNEL);
>> +	if (!sfw)
>> +		return -ENOMEM;
>> +
>> +	memcpy(sfw, fw, sizeof(*sfw));
>Is it safe to use standard memcpy() here when fw could be an I/O memory
>pointer?
>When the KPU profile is loaded from the firmware database, fw points
>directly to an __iomem mapping via ioremap_wc(). On ARM64 architectures,
>using standard memcpy() on device memory can cause unaligned or wide
>SIMD/NEON load instructions, resulting in a Synchronous Abort and kernel
>panic.
>Should this code use memcpy_fromio() instead to handle the device memory
>safely?
>> +
>>  	profile->custom = 1;
>> -	profile->name = fw->name;
>> +	profile->name = sfw->name;
>>  	profile->version = le64_to_cpu(fw->version);
>> -	profile->mcam_kex_prfl.mkex = &fw->mkex;
>> -	profile->lt_def = &fw->lt_def;
>[ ... ]

The 'fw' pointer references a standard
system memory shadow buffer allocated via the firmware subsystem interface
(e.g., request_firmware), not a direct MMIO or __iomem mapped peripheral
region.