netdev - Re: [PATCH net-next v10 05/12] virtio_net: Query and set flow filter caps

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0483aaba-0b93-41d7-bf09-5430b5520395@nvidia.com>
Date: Mon, 17 Nov 2025 11:49:54 -0600
From: Dan Jurgens <danielj@...dia.com>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, mst@...hat.com, jasowang@...hat.com,
 pabeni@...hat.com, virtualization@...ts.linux.dev, parav@...dia.com,
 shshitrit@...dia.com, yohadt@...dia.com, xuanzhuo@...ux.alibaba.com,
 eperezma@...hat.com, shameerali.kolothum.thodi@...wei.com, jgg@...pe.ca,
 kevin.tian@...el.com, kuba@...nel.org, andrew+netdev@...n.ch,
 edumazet@...gle.com
Subject: Re: [PATCH net-next v10 05/12] virtio_net: Query and set flow filter
 caps

On 11/17/25 11:16 AM, Simon Horman wrote:
> On Wed, Nov 12, 2025 at 01:34:28PM -0600, Daniel Jurgens wrote:
> 
> ...
> 
>> +static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
>> +{
>> +	size_t ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data) +
>> +			      sizeof(struct virtio_net_ff_selector) *
>> +			      VIRTIO_NET_FF_MASK_TYPE_MAX;
>> +	struct virtio_admin_cmd_query_cap_id_result *cap_id_list;
>> +	struct virtio_net_ff_selector *sel;
>> +	size_t real_ff_mask_size;
>> +	int err;
>> +	int i;
>> +
>> +	if (!vdev->config->admin_cmd_exec)
>> +		return -EOPNOTSUPP;
>> +
>> +	cap_id_list = kzalloc(sizeof(*cap_id_list), GFP_KERNEL);
>> +	if (!cap_id_list)
>> +		return -ENOMEM;
>> +
>> +	err = virtio_admin_cap_id_list_query(vdev, cap_id_list);
>> +	if (err)
>> +		goto err_cap_list;
>> +
>> +	if (!(VIRTIO_CAP_IN_LIST(cap_id_list,
>> +				 VIRTIO_NET_FF_RESOURCE_CAP) &&
>> +	      VIRTIO_CAP_IN_LIST(cap_id_list,
>> +				 VIRTIO_NET_FF_SELECTOR_CAP) &&
>> +	      VIRTIO_CAP_IN_LIST(cap_id_list,
>> +				 VIRTIO_NET_FF_ACTION_CAP))) {
>> +		err = -EOPNOTSUPP;
>> +		goto err_cap_list;
>> +	}
>> +
>> +	ff->ff_caps = kzalloc(sizeof(*ff->ff_caps), GFP_KERNEL);
>> +	if (!ff->ff_caps) {
>> +		err = -ENOMEM;
>> +		goto err_cap_list;
>> +	}
>> +
>> +	err = virtio_admin_cap_get(vdev,
>> +				   VIRTIO_NET_FF_RESOURCE_CAP,
>> +				   ff->ff_caps,
>> +				   sizeof(*ff->ff_caps));
>> +
>> +	if (err)
>> +		goto err_ff;
>> +
>> +	/* VIRTIO_NET_FF_MASK_TYPE start at 1 */
>> +	for (i = 1; i <= VIRTIO_NET_FF_MASK_TYPE_MAX; i++)
>> +		ff_mask_size += get_mask_size(i);
>> +
>> +	ff->ff_mask = kzalloc(ff_mask_size, GFP_KERNEL);
>> +	if (!ff->ff_mask) {
>> +		err = -ENOMEM;
>> +		goto err_ff;
>> +	}
>> +
>> +	err = virtio_admin_cap_get(vdev,
>> +				   VIRTIO_NET_FF_SELECTOR_CAP,
>> +				   ff->ff_mask,
>> +				   ff_mask_size);
>> +
>> +	if (err)
>> +		goto err_ff_mask;
>> +
>> +	ff->ff_actions = kzalloc(sizeof(*ff->ff_actions) +
>> +					VIRTIO_NET_FF_ACTION_MAX,
>> +					GFP_KERNEL);
>> +	if (!ff->ff_actions) {
>> +		err = -ENOMEM;
>> +		goto err_ff_mask;
>> +	}
>> +
>> +	err = virtio_admin_cap_get(vdev,
>> +				   VIRTIO_NET_FF_ACTION_CAP,
>> +				   ff->ff_actions,
>> +				   sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
>> +
>> +	if (err)
>> +		goto err_ff_action;
>> +
>> +	err = virtio_admin_cap_set(vdev,
>> +				   VIRTIO_NET_FF_RESOURCE_CAP,
>> +				   ff->ff_caps,
>> +				   sizeof(*ff->ff_caps));
>> +	if (err)
>> +		goto err_ff_action;
>> +
>> +	real_ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
>> +	sel = (void *)&ff->ff_mask->selectors[0];
>> +
>> +	for (i = 0; i < ff->ff_mask->count; i++) {
>> +		if (sel->length > MAX_SEL_LEN) {
>> +			err = -EINVAL;
>> +			goto err_ff_action;
>> +		}
>> +		real_ff_mask_size += sizeof(struct virtio_net_ff_selector) + sel->length;
>> +		sel = (void *)sel + sizeof(*sel) + sel->length;
>> +	}
> 
> Hi Daniel,
> 
> I'm not sure that the bounds checking in the loop above is adequate.
> For example, if ff->ff_mask->count is larger than expected.
> Or sel->length returns MAX_SEL_LEN each time then it seems
> than sel could overrun the space allocated for ff->ff_mask.
> 
> Flagged by Claude Code with https://github.com/masoncl/review-prompts/
> 

I can also bound the loop by VIRTIO_NET_FF_MASK_TYPE_MAX. I'll also
address your comments about classifier and rules limits on patch 7 here,
by checking the rules and classifier limits are > 0.

I'll wait to push a new version until I hear back from Michael about the
threading comment he made on the cover letter.