Re: [PATCH v4 4/6] net/eth: Check rt_hdr size before casting to ip6_ext_hdr

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 3/10/21 10:36 AM, Miroslav Rezanina wrote:
> ----- Original Message -----
>> From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
>> To: qemu-devel@nongnu.org
>> Cc: "Laurent Vivier" <lvivier@redhat.com>, "Dmitry Fleytman" <dmitry.fleytman@gmail.com>, "Miroslav Rezanina"
>> <mrezanin@redhat.com>, "Li Qiang" <liq3ea@gmail.com>, "Paolo Bonzini" <pbonzini@redhat.com>, "Jason Wang"
>> <jasowang@redhat.com>, "Thomas Huth" <thuth@redhat.com>, "Alexander Bulekov" <alxndr@bu.edu>, "Stefano Garzarella"
>> <sgarzare@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-stable@nongnu.org
>> Sent: Tuesday, March 9, 2021 7:27:07 PM
>> Subject: [PATCH v4 4/6] net/eth: Check rt_hdr size before casting to ip6_ext_hdr
>>
>> Do not cast our ip6_ext_hdr pointer to ip6_ext_hdr_routing if there
>> isn't enough data in the buffer for a such structure.
>>

>> FWIW GCC 11 similarly reported:
>>
>>   net/eth.c: In function 'eth_parse_ipv6_hdr':
>>   net/eth.c:410:15: error: array subscript 'struct ip6_ext_hdr_routing[0]' is
>>   partly outside array bounds of 'struct ip6_ext_hdr[1]'
>>   [-Werror=array-bounds]
>>     410 |     if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
>>         |          ~~~~~^~~~~~~
>>   net/eth.c:485:24: note: while referencing 'ext_hdr'
>>     485 |     struct ip6_ext_hdr ext_hdr;
>>         |                        ^~~~~~~
>>   net/eth.c:410:38: error: array subscript 'struct ip6_ext_hdr_routing[0]' is
>>   partly outside array bounds of 'struct ip6_ext_hdr[1]'
>>   [-Werror=array-bounds]
>>     410 |     if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
>>         |                                 ~~~~~^~~~~~~~~
>>   net/eth.c:485:24: note: while referencing 'ext_hdr'
>>     485 |     struct ip6_ext_hdr ext_hdr;
>>         |                        ^~~~~~~
>>
>> Cc: qemu-stable@nongnu.org
>> Buglink: https://bugs.launchpad.net/qemu/+bug/1879531
>> Reported-by: Alexander Bulekov <alxndr@bu.edu>
>> Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
>> Fixes: eb700029c78 ("net_pkt: Extend packet abstraction as required by e1000e
>> functionality")
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  net/eth.c                      |  7 ++++-
>>  tests/qtest/fuzz-e1000e-test.c | 53 ++++++++++++++++++++++++++++++++++
>>  MAINTAINERS                    |  1 +
>>  tests/qtest/meson.build        |  1 +
>>  4 files changed, 61 insertions(+), 1 deletion(-)
>>  create mode 100644 tests/qtest/fuzz-e1000e-test.c
>>
>> diff --git a/net/eth.c b/net/eth.c
>> index 77af2b673bb..f0c8dfe8df7 100644
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -406,7 +406,12 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int
>> pkt_frags,
>>                          struct in6_address *dst_addr)
>>  {
>>      size_t input_size = iov_size(pkt, pkt_frags);
>> -    struct ip6_ext_hdr_routing *rthdr = (struct ip6_ext_hdr_routing *)
>> ext_hdr;
>> +    struct ip6_ext_hdr_routing *rthdr;
>> +
>> +    if (input_size < ext_hdr_offset + sizeof(*rthdr)) {
>> +        return false;
>> +    }
>> +    rthdr = (struct ip6_ext_hdr_routing *) ext_hdr;
> 
> Hi Philippe,
> 
> you're introducing the problem you're trying to fix here. This line
> cause warning on GCC 11 and so the build fail with --enable-werror.

Oh, I thought we had switched to testing on Fedora rawhide, but this
never made it to mainstream [*], which is why my testing didn't notice
this was not fixing.

I'll keep working on this, thanks.

[*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg774720.html