[PATCH net] packet: fix reserve calculation

[PATCH net] packet: fix reserve calculation

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.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@mellanox.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
 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

Re: [PATCH net] packet: fix reserve calculation

[David Miller]

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Thu, 24 May 2018 18:10:30 -0400

> From: Willem de Bruijn <willemb@google.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@mellanox.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Applied and queued up for -stable.

Re: [PATCH net] packet: fix reserve calculation

[Lennert Buytenhek]

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

> From: Willem de Bruijn <willemb@google.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@mellanox.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.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@wantstofly.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