Re: [PATCH v13 net-next 4/9] devlink: Implement devlink param multi attribute nested data values

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-05-11 at 09:09:18, Ratheesh Kannoth (rkannoth@marvell.com) wrote:
> From: Saeed Mahameed <saeedm@nvidia.com>
>
> Devlink param value attribute is not defined since devlink is handling
> the value validating and parsing internally, this allows us to implement
> multi attribute values without breaking any policies.
>

>> diff --git a/net/devlink/param.c b/net/devlink/param.c
>> index 1a196d3a843db..4cc479bd019fd 100644
>> --- a/net/devlink/param.c
>> +++ b/net/devlink/param.c
>> @@ -252,6 +252,11 @@ devlink_nl_param_value_put(struct sk_buff *msg, enum devlink_param_type type,
>>  				return -EMSGSIZE;
>>  		}
>>  		break;
>> +	case DEVLINK_PARAM_TYPE_U64_ARRAY:
>> +		for (int i = 0; i < val->u64arr.size; i++)
>Can this loop read out of bounds of val->u64arr.val[]?
>There is no bounds check against __DEVLINK_PARAM_MAX_ARRAY_SIZE. If
>val->u64arr.size contains garbage or a maliciously large value, this will read
>past the end of the u64arr.val array.
>Could this leak adjacent kernel memory to userspace via nla_put_uint()?
>> +			if (nla_put_uint(msg, nla_type, val->u64arr.val[i]))
>> +				return -EMSGSIZE;
>> +		break;
>>  	}
>>  	return 0;
>>  }

pw-bot: changes-requested
ACK.

>[ ... ]
>> +	param_value = kcalloc(DEVLINK_PARAM_CMODE_MAX + 1,
>> +			      sizeof(*param_value), GFP_KERNEL);
>> +	if (!param_value) {
>> +		kfree(default_value);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	ctx = kmalloc_obj(*ctx);
>Does this allocation need to be zero-initialized?
>Since kmalloc_obj() does not zero the memory, if a driver's get callback
>successfully returns but fails to initialize ctx->val.u64arr.size, the struct
>will contain uninitialized kernel heap garbage.
>This data is then copied into the param_value array and eventually read by
>devlink_nl_param_value_put(), potentially causing the out-of-bounds read
>mentioned above.
>> +	if (!ctx) {
>> +		kfree(param_value);
>> +		kfree(default_value);
>> +		return -ENOMEM;
>> +	}
There is no issue with these uninitialized kernel heap garbage.