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

[Dan Jurgens <danielj@nvidia.com>]

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.