git.openwrt.org Git - openwrt/openwrt.git/blob

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: "Jason A. Donenfeld" <Jason@zx2c4.com>
   3 Date: Wed, 18 Mar 2020 18:30:45 -0600
   4 Subject: [PATCH] wireguard: queueing: account for skb->protocol==0
   5
   6 commit a5588604af448664e796daf3c1d5a4523c60667b upstream.
   7
   8 We carry out checks to the effect of:
   9
  10   if (skb->protocol != wg_examine_packet_protocol(skb))
  11     goto err;
  12
  13 By having wg_skb_examine_untrusted_ip_hdr return 0 on failure, this
  14 means that the check above still passes in the case where skb->protocol
  15 is zero, which is possible to hit with AF_PACKET:
  16
  17   struct sockaddr_pkt saddr = { .spkt_device = "wg0" };
  18   unsigned char buffer[5] = { 0 };
  19   sendto(socket(AF_PACKET, SOCK_PACKET, /* skb->protocol = */ 0),
  20          buffer, sizeof(buffer), 0, (const struct sockaddr *)&saddr, sizeof(saddr));
  21
  22 Additional checks mean that this isn't actually a problem in the code
  23 base, but I could imagine it becoming a problem later if the function is
  24 used more liberally.
  25
  26 I would prefer to fix this by having wg_examine_packet_protocol return a
  27 32-bit ~0 value on failure, which will never match any value of
  28 skb->protocol, which would simply change the generated code from a mov
  29 to a movzx. However, sparse complains, and adding __force casts doesn't
  30 seem like a good idea, so instead we just add a simple helper function
  31 to check for the zero return value. Since wg_examine_packet_protocol
  32 itself gets inlined, this winds up not adding an additional branch to
  33 the generated code, since the 0 return value already happens in a
  34 mergable branch.
  35
  36 Reported-by: Fabian Freyer <fabianfreyer@radicallyopensecurity.com>
  37 Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
  38 Signed-off-by: David S. Miller <davem@davemloft.net>
  39 Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
  40 ---
  41  drivers/net/wireguard/device.c   | 2 +-
  42  drivers/net/wireguard/queueing.h | 8 +++++++-
  43  drivers/net/wireguard/receive.c  | 4 ++--
  44  3 files changed, 10 insertions(+), 4 deletions(-)
  45
  46 --- a/drivers/net/wireguard/device.c
  47 +++ b/drivers/net/wireguard/device.c
  48 @@ -122,7 +122,7 @@ static netdev_tx_t wg_xmit(struct sk_buf
  49         u32 mtu;
  50         int ret;
  51
  52 -       if (unlikely(wg_skb_examine_untrusted_ip_hdr(skb) != skb->protocol)) {
  53 +       if (unlikely(!wg_check_packet_protocol(skb))) {
  54                 ret = -EPROTONOSUPPORT;
  55                 net_dbg_ratelimited("%s: Invalid IP packet\n", dev->name);
  56                 goto err;
  57 --- a/drivers/net/wireguard/queueing.h
  58 +++ b/drivers/net/wireguard/queueing.h
  59 @@ -66,7 +66,7 @@ struct packet_cb {
  60  #define PACKET_PEER(skb) (PACKET_CB(skb)->keypair->entry.peer)
  61
  62  /* Returns either the correct skb->protocol value, or 0 if invalid. */
  63 -static inline __be16 wg_skb_examine_untrusted_ip_hdr(struct sk_buff *skb)
  64 +static inline __be16 wg_examine_packet_protocol(struct sk_buff *skb)
  65  {
  66         if (skb_network_header(skb) >= skb->head &&
  67             (skb_network_header(skb) + sizeof(struct iphdr)) <=
  68 @@ -81,6 +81,12 @@ static inline __be16 wg_skb_examine_untr
  69         return 0;
  70  }
  71
  72 +static inline bool wg_check_packet_protocol(struct sk_buff *skb)
  73 +{
  74 +       __be16 real_protocol = wg_examine_packet_protocol(skb);
  75 +       return real_protocol && skb->protocol == real_protocol;
  76 +}
  77 +
  78  static inline void wg_reset_packet(struct sk_buff *skb)
  79  {
  80         skb_scrub_packet(skb, true);
  81 --- a/drivers/net/wireguard/receive.c
  82 +++ b/drivers/net/wireguard/receive.c
  83 @@ -56,7 +56,7 @@ static int prepare_skb_header(struct sk_
  84         size_t data_offset, data_len, header_len;
  85         struct udphdr *udp;
  86
  87 -       if (unlikely(wg_skb_examine_untrusted_ip_hdr(skb) != skb->protocol ||
  88 +       if (unlikely(!wg_check_packet_protocol(skb) ||
  89                      skb_transport_header(skb) < skb->head ||
  90                      (skb_transport_header(skb) + sizeof(struct udphdr)) >
  91                              skb_tail_pointer(skb)))
  92 @@ -388,7 +388,7 @@ static void wg_packet_consume_data_done(
  93          */
  94         skb->ip_summed = CHECKSUM_UNNECESSARY;
  95         skb->csum_level = ~0; /* All levels */
  96 -       skb->protocol = wg_skb_examine_untrusted_ip_hdr(skb);
  97 +       skb->protocol = wg_examine_packet_protocol(skb);
  98         if (skb->protocol == htons(ETH_P_IP)) {
  99                 len = ntohs(ip_hdr(skb)->tot_len);
 100                 if (unlikely(len < sizeof(struct iphdr)))