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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <a3aec539-9468-4273-b7fa-6705331c9b92@nvidia.com>
Date: Tue, 3 Feb 2026 15:40:24 -0600
From: Dan Jurgens <danielj@...dia.com>
To: Paolo Abeni <pabeni@...hat.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

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.

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;
                }



> ---
> 
> Note that the AI review additionally reported a possible leak on xarray,
> but I think it got confused possibly because it run out of token and
> mixed-up the patch context.
> 

Yes, I saw this and it didn't make sense.

> /P
>