netdev - [PATCH v2 0/4] Check size of packets before sending

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-next>] [day] [month] [year] [list]

Date:   Thu, 25 Jan 2018 15:31:05 +1100
From:   Daniel Axtens <dja@...ens.net>
To:     netdev@...r.kernel.org
Cc:     Daniel Axtens <dja@...ens.net>, Jason Wang <jasowang@...hat.com>,
        Pravin Shelar <pshelar@....org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Manish.Chopra@...ium.com, dev@...nvswitch.org
Subject: [PATCH v2 0/4] Check size of packets before sending

There are a few ways we can send packets that are too large to a
network driver.

When non-GSO packets are forwarded, we validate their size, based on
the MTU of the destination device. However, when GSO packets are
forwarded, we do not validate their size. We implicitly assume that
when they are segmented, the resultant packets will be correctly
sized.

This is not always the case.

We observed a case where a packet received on an ibmveth device had a
GSO size of around 10kB. This was forwarded by Open vSwitch to a bnx2x
device, where it caused a firmware assert. This is described in detail
at [0] and was the genesis of this series.

Rather than fixing this in the driver, this series fixes the
core path. It does it in 2 steps:

 1) make is_skb_forwardable check GSO packets - this catches bridges

 2) make validate_xmit_skb check the size of all packets, so as to
    catch everything else (e.g. macvlan, tc mired, OVS)

I am a bit nervous about how this series will interact with nested
VLANs, as the existing code only allows for one VLAN_HLEN. (Previously
these packets would sail past unchecked.) But I thought it would be
prudent to get more eyes on this sooner rather than later.

Thanks,
Daniel

v1: https://www.spinics.net/lists/netdev/msg478634.html
Changes in v2:

 - improve names, thanks Marcelo Ricardo Leitner

 - add check to xmit_validate_skb; thanks to everyone who participated
   in the discussion.

 - drop extra check in Open vSwitch. Bad packets will be caught by
   validate_xmit_skb for now and we can come back and add it later if
   OVS people would like the extra logging.

[0]: https://patchwork.ozlabs.org/patch/859410/

Cc: Jason Wang <jasowang@...hat.com>
Cc: Pravin Shelar <pshelar@....org>
Cc: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc: Manish.Chopra@...ium.com
Cc: dev@...nvswitch.org

Daniel Axtens (4):
  net: rename skb_gso_validate_mtu -> skb_gso_validate_network_len
  net: move skb_gso_mac_seglen to skbuff.h
  net: is_skb_forwardable: check the size of GSO segments
  net: check the size of a packet in validate_xmit_skb

 include/linux/skbuff.h                  | 18 ++++++++-
 net/core/dev.c                          | 24 ++++++++----
 net/core/skbuff.c                       | 66 ++++++++++++++++++++++++++-------
 net/ipv4/ip_forward.c                   |  2 +-
 net/ipv4/ip_output.c                    |  2 +-
 net/ipv4/netfilter/nf_flow_table_ipv4.c |  2 +-
 net/ipv6/ip6_output.c                   |  2 +-
 net/ipv6/netfilter/nf_flow_table_ipv6.c |  2 +-
 net/mpls/af_mpls.c                      |  2 +-
 net/sched/sch_tbf.c                     | 10 -----
 net/xfrm/xfrm_device.c                  |  2 +-
 11 files changed, 93 insertions(+), 39 deletions(-)

-- 
2.14.1