• Jason A. Donenfeld's avatar
    wireguard: queueing: account for skb->protocol==0 · a5588604
    Jason A. Donenfeld authored
    We carry out checks to the effect of:
    
      if (skb->protocol != wg_examine_packet_protocol(skb))
        goto err;
    
    By having wg_skb_examine_untrusted_ip_hdr return 0 on failure, this
    means that the check above still passes in the case where skb->protocol
    is zero, which is possible to hit with AF_PACKET:
    
      struct sockaddr_pkt saddr = { .spkt_device = "wg0" };
      unsigned char buffer[5] = { 0 };
      sendto(socket(AF_PACKET, SOCK_PACKET, /* skb->protocol = */ 0),
             buffer, sizeof(buffer), 0, (const struct sockaddr *)&saddr, sizeof(saddr));
    
    Additional checks mean that this isn't actually a problem in the code
    base, but I could imagine it becoming a problem later if the function is
    used more liberally.
    
    I would prefer to fix this by having wg_examine_packet_protocol return a
    32-bit ~0 value on failure, which will never match any value of
    skb->protocol, which would simply change the generated code from a mov
    to a movzx. However, sparse complains, and adding __force casts doesn't
    seem like a good idea, so instead we just add a simple helper function
    to check for the zero return value. Since wg_examine_packet_protocol
    itself gets inlined, this winds up not adding an additional branch to
    the generated code, since the 0 return value already happens in a
    mergable branch.
    Reported-by: default avatarFabian Freyer <fabianfreyer@radicallyopensecurity.com>
    Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    a5588604
receive.c 17.7 KB