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] [day] [month] [year] [list]

Message-ID: <aRxASQDUgfhKo6ki@horms.kernel.org>
Date: Tue, 18 Nov 2025 09:45:45 +0000
From: Simon Horman <horms@...nel.org>
To: Dan Jurgens <danielj@...dia.com>
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 Mon, Nov 17, 2025 at 03:21:08PM -0600, Dan Jurgens wrote:
> On 11/17/25 3:18 PM, Simon Horman wrote:
> > On Mon, Nov 17, 2025 at 11:49:54AM -0600, Dan Jurgens wrote:
> >> On 11/17/25 11:16 AM, Simon Horman wrote:
> >>> On Wed, Nov 12, 2025 at 01:34:28PM -0600, Daniel Jurgens wrote:
> > 
> > ...
> > 
> >>>> +	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.
> > 
> > Thanks.
> > 
> > I think that even if the loop is bounded there is still (a much smaller)
> > scope for an overflow. This is because selectors isn't large enough for
> > VIRTIO_NET_FF_MASK_TYPE_MAX entries if all of them have length ==
> > MAX_SEL_LEN.
> > 
> >>
> >> I'll wait to push a new version until I hear back from Michael about the
> >> threading comment he made on the cover letter.
> >>
> 
> I actually moved the if (real_ff_mask_size > ff_mask_size) check into
> the loop, before updating the selector pointer.

Sounds good, thanks.