| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| From: "Jason A. Donenfeld" <Jason@zx2c4.com> |
| Date: Fri, 14 Feb 2020 23:57:22 +0100 |
| Subject: [PATCH] wireguard: send: account for mtu=0 devices |
| |
| commit 175f1ca9a9ed8689d2028da1a7c624bb4fb4ff7e upstream. |
| |
| It turns out there's an easy way to get packets queued up while still |
| having an MTU of zero, and that's via persistent keep alive. This commit |
| makes sure that in whatever condition, we don't wind up dividing by |
| zero. Note that an MTU of zero for a wireguard interface is something |
| quasi-valid, so I don't think the correct fix is to limit it via |
| min_mtu. This can be reproduced easily with: |
| |
| ip link add wg0 type wireguard |
| ip link add wg1 type wireguard |
| ip link set wg0 up mtu 0 |
| ip link set wg1 up |
| wg set wg0 private-key <(wg genkey) |
| wg set wg1 listen-port 1 private-key <(wg genkey) peer $(wg show wg0 public-key) |
| wg set wg0 peer $(wg show wg1 public-key) persistent-keepalive 1 endpoint 127.0.0.1:1 |
| |
| However, while min_mtu=0 seems fine, it makes sense to restrict the |
| max_mtu. This commit also restricts the maximum MTU to the greatest |
| number for which rounding up to the padding multiple won't overflow a |
| signed integer. Packets this large were always rejected anyway |
| eventually, due to checks deeper in, but it seems more sound not to even |
| let the administrator configure something that won't work anyway. |
| |
| We use this opportunity to clean up this function a bit so that it's |
| clear which paths we're expecting. |
| |
| Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> |
| Cc: Eric Dumazet <eric.dumazet@gmail.com> |
| Reviewed-by: Eric Dumazet <edumazet@google.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> |
| --- |
| drivers/net/wireguard/device.c | 7 ++++--- |
| drivers/net/wireguard/send.c | 16 +++++++++++----- |
| 2 files changed, 15 insertions(+), 8 deletions(-) |
| |
| --- a/drivers/net/wireguard/device.c |
| +++ b/drivers/net/wireguard/device.c |
| @@ -258,6 +258,8 @@ static void wg_setup(struct net_device * |
| enum { WG_NETDEV_FEATURES = NETIF_F_HW_CSUM | NETIF_F_RXCSUM | |
| NETIF_F_SG | NETIF_F_GSO | |
| NETIF_F_GSO_SOFTWARE | NETIF_F_HIGHDMA }; |
| + const int overhead = MESSAGE_MINIMUM_LENGTH + sizeof(struct udphdr) + |
| + max(sizeof(struct ipv6hdr), sizeof(struct iphdr)); |
| |
| dev->netdev_ops = &netdev_ops; |
| dev->hard_header_len = 0; |
| @@ -271,9 +273,8 @@ static void wg_setup(struct net_device * |
| dev->features |= WG_NETDEV_FEATURES; |
| dev->hw_features |= WG_NETDEV_FEATURES; |
| dev->hw_enc_features |= WG_NETDEV_FEATURES; |
| - dev->mtu = ETH_DATA_LEN - MESSAGE_MINIMUM_LENGTH - |
| - sizeof(struct udphdr) - |
| - max(sizeof(struct ipv6hdr), sizeof(struct iphdr)); |
| + dev->mtu = ETH_DATA_LEN - overhead; |
| + dev->max_mtu = round_down(INT_MAX, MESSAGE_PADDING_MULTIPLE) - overhead; |
| |
| SET_NETDEV_DEVTYPE(dev, &device_type); |
| |
| --- a/drivers/net/wireguard/send.c |
| +++ b/drivers/net/wireguard/send.c |
| @@ -143,16 +143,22 @@ static void keep_key_fresh(struct wg_pee |
| |
| static unsigned int calculate_skb_padding(struct sk_buff *skb) |
| { |
| + unsigned int padded_size, last_unit = skb->len; |
| + |
| + if (unlikely(!PACKET_CB(skb)->mtu)) |
| + return ALIGN(last_unit, MESSAGE_PADDING_MULTIPLE) - last_unit; |
| + |
| /* We do this modulo business with the MTU, just in case the networking |
| * layer gives us a packet that's bigger than the MTU. In that case, we |
| * wouldn't want the final subtraction to overflow in the case of the |
| - * padded_size being clamped. |
| + * padded_size being clamped. Fortunately, that's very rarely the case, |
| + * so we optimize for that not happening. |
| */ |
| - unsigned int last_unit = skb->len % PACKET_CB(skb)->mtu; |
| - unsigned int padded_size = ALIGN(last_unit, MESSAGE_PADDING_MULTIPLE); |
| + if (unlikely(last_unit > PACKET_CB(skb)->mtu)) |
| + last_unit %= PACKET_CB(skb)->mtu; |
| |
| - if (padded_size > PACKET_CB(skb)->mtu) |
| - padded_size = PACKET_CB(skb)->mtu; |
| + padded_size = min(PACKET_CB(skb)->mtu, |
| + ALIGN(last_unit, MESSAGE_PADDING_MULTIPLE)); |
| return padded_size - last_unit; |
| } |
| |