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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 21 Feb 2019 12:28:03 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Maxim Mikityanskiy <maximmi@...lanox.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 net-next v2 1/7] net: Don't set transport offset to
 invalid value

On Thu, Feb 21, 2019 at 7:40 AM Maxim Mikityanskiy <maximmi@...lanox.com> wrote:
>
> If the socket was created with socket(AF_PACKET, SOCK_RAW, 0),
> skb->protocol will be unset, __skb_flow_dissect() will fail, and
> skb_probe_transport_header() will fall back to the offset_hint, making
> the resulting skb_transport_offset incorrect.
>
> If, however, there is no transport header in the packet,
> transport_header shouldn't be set to an arbitrary value.
>
> Fix it by leaving the transport offset unset if it couldn't be found, to
> be explicit rather than to fill it with some wrong value. It changes the
> behavior, but if some code relied on the old behavior, it would be
> broken anyway, as the old one is incorrect.
>
> Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com>

qdisc_pkt_len_init also expects skb_transport_header(skb) to always be
set for gso packets.

Once net is merged into net-next, commit d5be7f632bad ("net: validate
untrusted gso packets without csum offload") will ensure that packets
that fail flow dissection do not make it into the stack. But we have
to skip dissection in some cases, like tun [1].

I think we need to add a check in qdisc_pkt_len_init to skip the gso
size estimation branch if !skb_transport_header_was_set(skb).

Otherwise this patch set looks good to me. To avoid resubmitting
everything we can fix up the qdisc_pkt_len_init in a follow-up, in
which case I'm happy to add my Acked-by to this series.

[1] http://patchwork.ozlabs.org/patch/1044429/

Powered by blists - more mailing lists