netdev - Re: [PATCH net] packet: fix reserve calculation

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180602163521.GE1861@wantstofly.org>
Date:   Sat, 2 Jun 2018 19:35:21 +0300
From:   Lennert Buytenhek <buytenh@...tstofly.org>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] packet: fix reserve calculation

On Thu, May 24, 2018 at 06:10:30PM -0400, Willem de Bruijn wrote:

> From: Willem de Bruijn <willemb@...gle.com>
> 
> Commit b84bbaf7a6c8 ("packet: in packet_snd start writing at link
> layer allocation") ensures that packet_snd always starts writing
> the link layer header in reserved headroom allocated for this
> purpose.
> 
> This is needed because packets may be shorter than hard_header_len,
> in which case the space up to hard_header_len may be zeroed. But
> that necessary padding is not accounted for in skb->len.
> 
> The fix, however, is buggy. It calls skb_push, which grows skb->len
> when moving skb->data back. But in this case packet length should not
> change.
> 
> Instead, call skb_reserve, which moves both skb->data and skb->tail
> back, without changing length.
> 
> Fixes: b84bbaf7a6c8 ("packet: in packet_snd start writing at link layer allocation")
> Reported-by: Tariq Toukan <tariqt@...lanox.com>
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@...gle.com>

After upgrading my router from 4.16.11 to 4.16.12, it is failing to
obtain a DHCP lease from my ISP, as it started sending out DHCP queries
with 14 bytes of junk at the end (which is presumably causing RX csum
failures on the DHCP server end):

 13:08:39.292667 IP (tos 0x10, ttl 128, id 0, offset 0, flags [none], proto UDP (17), length 328)
     0.0.0.0.68 > 255.255.255.255.67: [udp sum ok] BOOTP/DHCP, Request from xx:xx:xx:xx:xx:xx, length 300, xid 0xxxxxxxxx, Flags [none] (0x0000)
[...]
-        0x0150:  0000 0000 0000
+        0x0150:  0000 0000 0000 e802 0000 0000 0000 e802
+        0x0160:  0000 0000

This seems to be caused by (the -stable backport of) b84bbaf7a6c8
("packet: in packet_snd start writing at link layer allocation") and
appears to have been fixed by this patch, as applying this patch to
4.16.12 makes DHCP work for me again.

Tested-by: Lennert Buytenhek <buytenh@...tstofly.org>



> ---
>  net/packet/af_packet.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index e9422fe45179..acb7b86574cd 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2911,7 +2911,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>  		if (unlikely(offset < 0))
>  			goto out_free;
>  	} else if (reserve) {
> -		skb_push(skb, reserve);
> +		skb_reserve(skb, -reserve);
>  	}
>  
>  	/* Returns -EFAULT on error */
> -- 
> 2.17.0.921.gf22659ad46-goog