We want to check fields from ip6_ext_hdr_routing structure
and if correct read the full in6_address. Let's directly check
if our iovec contains enough data for everything, else return
early.
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
net/eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/eth.c b/net/eth.c
index e870d02b0df..28cdc843a69 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -409,7 +409,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
size_t input_size = iov_size(pkt, pkt_frags);
size_t bytes_read;
- if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
+ if (input_size < ext_hdr_offset + sizeof(*rthdr) + sizeof(*dst_addr)) {
return false;
}
--
2.26.2
On Wed, Mar 10, 2021 at 05:01:33PM +0100, Philippe Mathieu-Daudé wrote:
>We want to check fields from ip6_ext_hdr_routing structure
>and if correct read the full in6_address. Let's directly check
>if our iovec contains enough data for everything, else return
>early.
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>---
> net/eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/net/eth.c b/net/eth.c
>index e870d02b0df..28cdc843a69 100644
>--- a/net/eth.c
>+++ b/net/eth.c
>@@ -409,7 +409,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
> size_t input_size = iov_size(pkt, pkt_frags);
> size_t bytes_read;
>
>- if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
>+ if (input_size < ext_hdr_offset + sizeof(*rthdr) + sizeof(*dst_addr)) {
> return false;
> }
If you have to respin, maybe we should also fix the offset in
iov_to_buf() in this patch and queue it for stable:
@@ -415,7 +415,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
bytes_read = iov_to_buf(pkt, pkt_frags,
- ext_hdr_offset + sizeof(*ext_hdr),
+ ext_hdr_offset + sizeof(*rthdr),
dst_addr, sizeof(*dst_addr));
return bytes_read == sizeof(*dst_addr);
Otherwise:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
On 3/10/21 5:53 PM, Stefano Garzarella wrote:
> On Wed, Mar 10, 2021 at 05:01:33PM +0100, Philippe Mathieu-Daudé wrote:
>> We want to check fields from ip6_ext_hdr_routing structure
>> and if correct read the full in6_address. Let's directly check
>> if our iovec contains enough data for everything, else return
>> early.
>>
>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> net/eth.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/eth.c b/net/eth.c
>> index e870d02b0df..28cdc843a69 100644
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -409,7 +409,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt,
>> int pkt_frags,
>> size_t input_size = iov_size(pkt, pkt_frags);
>> size_t bytes_read;
>>
>> - if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
>> + if (input_size < ext_hdr_offset + sizeof(*rthdr) +
>> sizeof(*dst_addr)) {
>> return false;
>> }
>
> If you have to respin, maybe we should also fix the offset in
> iov_to_buf() in this patch and queue it for stable:
>
> @@ -415,7 +415,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt,
> int pkt_frags,
>
> if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
> bytes_read = iov_to_buf(pkt, pkt_frags,
> - ext_hdr_offset + sizeof(*ext_hdr),
> + ext_hdr_offset + sizeof(*rthdr),
> dst_addr, sizeof(*dst_addr));
Oh, so we always screwed the address by 4 bytes...
This code never worked correctly :(
>
> return bytes_read == sizeof(*dst_addr);
>
> Otherwise:
>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
On 3/10/21 6:57 PM, Philippe Mathieu-Daudé wrote:
> On 3/10/21 5:53 PM, Stefano Garzarella wrote:
>> On Wed, Mar 10, 2021 at 05:01:33PM +0100, Philippe Mathieu-Daudé wrote:
>>> We want to check fields from ip6_ext_hdr_routing structure
>>> and if correct read the full in6_address. Let's directly check
>>> if our iovec contains enough data for everything, else return
>>> early.
>>>
>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> net/eth.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/eth.c b/net/eth.c
>>> index e870d02b0df..28cdc843a69 100644
>>> --- a/net/eth.c
>>> +++ b/net/eth.c
>>> @@ -409,7 +409,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt,
>>> int pkt_frags,
>>> size_t input_size = iov_size(pkt, pkt_frags);
>>> size_t bytes_read;
>>>
>>> - if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
>>> + if (input_size < ext_hdr_offset + sizeof(*rthdr) +
>>> sizeof(*dst_addr)) {
>>> return false;
>>> }
>>
>> If you have to respin, maybe we should also fix the offset in
>> iov_to_buf() in this patch and queue it for stable:
>>
>> @@ -415,7 +415,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt,
>> int pkt_frags,
>>
>> if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
>> bytes_read = iov_to_buf(pkt, pkt_frags,
>> - ext_hdr_offset + sizeof(*ext_hdr),
>> + ext_hdr_offset + sizeof(*rthdr),
>> dst_addr, sizeof(*dst_addr));
>
> Oh, so we always screwed the address by 4 bytes...
>
> This code never worked correctly :(
Confirmed with commit 4555ca6816c ("net: fix incorrect
argument to iov_to_buf") when it then returns incorrect
value until b2caa3b82ed ("net/eth: fix incorrect check
of iov_to_buf() return value") one year later.
On Wed, Mar 10, 2021 at 07:26:19PM +0100, Philippe Mathieu-Daudé wrote:
>On 3/10/21 6:57 PM, Philippe Mathieu-Daudé wrote:
>> On 3/10/21 5:53 PM, Stefano Garzarella wrote:
>>> On Wed, Mar 10, 2021 at 05:01:33PM +0100, Philippe Mathieu-Daudé wrote:
>>>> We want to check fields from ip6_ext_hdr_routing structure
>>>> and if correct read the full in6_address. Let's directly check
>>>> if our iovec contains enough data for everything, else return
>>>> early.
>>>>
>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>> net/eth.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/eth.c b/net/eth.c
>>>> index e870d02b0df..28cdc843a69 100644
>>>> --- a/net/eth.c
>>>> +++ b/net/eth.c
>>>> @@ -409,7 +409,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt,
>>>> int pkt_frags,
>>>> size_t input_size = iov_size(pkt, pkt_frags);
>>>> size_t bytes_read;
>>>>
>>>> - if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
>>>> + if (input_size < ext_hdr_offset + sizeof(*rthdr) +
>>>> sizeof(*dst_addr)) {
>>>> return false;
>>>> }
>>>
>>> If you have to respin, maybe we should also fix the offset in
>>> iov_to_buf() in this patch and queue it for stable:
>>>
>>> @@ -415,7 +415,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt,
>>> int pkt_frags,
>>>
>>> if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
>>> bytes_read = iov_to_buf(pkt, pkt_frags,
>>> - ext_hdr_offset + sizeof(*ext_hdr),
>>> + ext_hdr_offset + sizeof(*rthdr),
>>> dst_addr, sizeof(*dst_addr));
>>
>> Oh, so we always screwed the address by 4 bytes...
>>
>> This code never worked correctly :(
>
>Confirmed with commit 4555ca6816c ("net: fix incorrect
>argument to iov_to_buf") when it then returns incorrect
>value until b2caa3b82ed ("net/eth: fix incorrect check
>of iov_to_buf() return value") one year later.
>
Ooooh, I agree, it never worked but I have no idea how to test...
Thanks for fixing this code,
Stefano
© 2016 - 2025 Red Hat, Inc.