netdev - Re: [PATCH net-next v17 07/12] virtio_net: Implement layer 2 ethtool flow rules

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 for Android: free password hash cracker in your pocket

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

Message-ID: <eb700d8d-81e0-4c56-9cf2-48473404563b@redhat.com>
Date: Wed, 4 Feb 2026 09:40:15 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Dan Jurgens <danielj@...dia.com>, netdev@...r.kernel.org, mst@...hat.com,
 jasowang@...hat.com
Cc: virtualization@...ts.linux.dev, parav@...dia.com, shshitrit@...dia.com,
 yohadt@...dia.com, xuanzhuo@...ux.alibaba.com, eperezma@...hat.com,
 jgg@...pe.ca, kevin.tian@...el.com, kuba@...nel.org, andrew+netdev@...n.ch,
 edumazet@...gle.com
Subject: Re: [PATCH net-next v17 07/12] virtio_net: Implement layer 2 ethtool
 flow rules


Hi,

On 2/3/26 10:40 PM, Dan Jurgens wrote:
> On 2/3/26 3:19 AM, Paolo Abeni wrote:
>> Hi,
>>
>> The AI review reported a possible issue that looks valid to me.
>> Reporting the feedback manually because I think only one to the AI
>> remarks is valid, see below.
>>
>> On 2/2/26 6:05 PM, Daniel Jurgens wrote:
>>> +static bool validate_eth_mask(const struct virtnet_ff *ff,
>>> +			      const struct virtio_net_ff_selector *sel,
>>> +			      const struct virtio_net_ff_selector *sel_cap)
>>> +{
>>> +	bool partial_mask = !!(sel_cap->flags & VIRTIO_NET_FF_MASK_F_PARTIAL_MASK);
>>> +	struct ethhdr *cap, *mask;
>>> +	struct ethhdr zeros = {};
>>> +
>>> +	cap = (struct ethhdr *)&sel_cap->mask;
>>> +	mask = (struct ethhdr *)&sel->mask;
>>
>> This function casts sel_cap->mask to struct ethhdr * and accesses fields
>> at offsets 0, 6, and 12. Shouldn't there be validation that
>> sel_cap->length is at least sizeof(struct ethhdr) = 14 bytes?
>>
>> Looking at virtnet_ff_init() at line 6291, it only checks that
>> sel->length <= MAX_SEL_LEN (40 bytes) but doesn't enforce a minimum
>> length for the ETH selector type. If a device provides an ETH selector
>> capability with length < 14 bytes, won't validate_eth_mask() read beyond
>> the allocated mask array?
> 
> It won't read beyond the end of the array. When we retrieve the selector
> caps we make sure the that the size of all the selectors and their data
> is less than what we allocated. So I don't think this is really a bug.

[sorry for the latency]

I'm possibly lost, but I do see such check for classifier->selectors,
but I do not see them for ff->ff_mask->selectors, neither in
get_selector_cap() nor in virtio_admin_cap_get().

> I'll change the check here to be sel->length !=
> get_mask_size(sel->type). It'll fail in a clearer way if this is the case.
> 
>                 if (sel->length > MAX_SEL_LEN ||
>                     test_and_set_bit(sel->type, &sel_types)) {
>                         WARN_ON_ONCE(true);
>                         err = -EINVAL;
>                         goto err_ff_action;
>                 }

If you mean:

		if (sel->length != get_mask_size(sel->type) ||
                      test_and_set_bit(sel->type, &sel_types))

LGTM!

Thanks,

Paolo