Re: [PATCH v5 5/7] net/eth: Check iovec has enough data earlier

[Stefano Garzarella]

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