netdev - RE: [PATCH 1/7] net: Don't set transport offset to invalid value

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]

Date:   Thu, 17 Jan 2019 09:10:13 +0000
From:   Maxim Mikityanskiy <maximmi@...lanox.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Willem de Bruijn <willemb@...gle.com>,
        Jason Wang <jasowang@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: RE: [PATCH 1/7] net: Don't set transport offset to invalid value

> This is a lot of code change. This would do.
> 
> @@ -2434,8 +2434,6 @@ static inline void
> skb_probe_transport_header(struct sk_buff *skb,
> 
>         if (skb_flow_dissect_flow_keys_basic(skb, &keys, NULL, 0, 0, 0, 0))
>                 skb_set_transport_header(skb, keys.control.thoff);
> -       else
> -               skb_set_transport_header(skb, offset_hint);
>  }
> 
> Though leaving an unused argument is a bit ugly. For net-next, indeed
> better to clean up (please mark your patchset with net or net-next,
> btw)

It's for net-next (I'll resend with the correct mark), so I'll stick
with the current implementation.

> > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> > index 80aae3a32c2a..b49b6e56ca47 100644
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -1105,6 +1105,7 @@ static int xenvif_tx_submit(struct xenvif_queue
> *queue)
> >                 struct xen_netif_tx_request *txp;
> >                 u16 pending_idx;
> >                 unsigned data_len;
> > +               bool th_set;
> >
> >                 pending_idx = XENVIF_TX_CB(skb)->pending_idx;
> >                 txp = &queue->pending_tx_info[pending_idx].req;
> > @@ -1169,20 +1170,22 @@ static int xenvif_tx_submit(struct xenvif_queue
> *queue)
> >                         continue;
> >                 }
> >
> > -               skb_probe_transport_header(skb, 0);
> > +               th_set = skb_try_probe_transport_header(skb);
> 
> Can use skb_transport_header_was_set(). Then at least there is no need
> to change the function's return value.

I suppose this comment relates to the previous one, and if we do it for
net-next, it's fine to make change I made, isn't it?

Thanks for reviewing.