Regression Description:

Depending on the options specified for the GRE tunnel device, small
packets may be dropped. This occurs because the pskb_network_may_pull
function fails due to the packet's insufficient length.

For example, if only the okey option is specified for the tunnel device,
original (before encapsulation) packets smaller than 28 bytes (including
the IPv4 header) will be dropped. This happens because the required
length is calculated relative to the network header, not the skb->head.

Here is how the required length is computed and checked:

* The pull_len variable is set to 28 bytes, consisting of:
  * IPv4 header: 20 bytes
  * GRE header with Key field: 8 bytes

* The pskb_network_may_pull function adds the network offset, shifting
the checkable space further to the beginning of the network header and
extending it to the beginning of the packet. As a result, the end of
the checkable space occurs beyond the actual end of the packet.

Instead of ensuring that 28 bytes are present in skb->head, the function
is requesting these 28 bytes starting from the network header. For small
packets, this requested length exceeds the actual packet size, causing
the check to fail and the packets to be dropped.

This issue affects both locally originated and forwarded packets in
DMVPN-like setups.

How to reproduce (for local originated packets):

  ip link add dev gre1 type gre ikey 1.9.8.4 okey 1.9.8.4 \
          local <your-ip> remote 0.0.0.0

  ip link set mtu 1400 dev gre1
  ip link set up dev gre1
  ip address add 192.168.13.1/24 dev gre1
  ip neighbor add 192.168.13.2 lladdr <remote-ip> dev gre1
  ping -s 1374 -c 10 192.168.13.2
  tcpdump -vni gre1
  tcpdump -vni <your-ext-iface> 'ip proto 47'
  ip -s -s -d link show dev gre1

Solution:

Use the pskb_may_pull function instead the pskb_network_may_pull.

Fixes: 80d875cfc9d3 ("ipv4: ip_gre: Avoid skb_pull() failure in ipgre_xmit()")

Signed-off-by: Anton Danilov <littlesmilingcloud@gmail.com>

---
v2 -> v3 :
- More accurate and detailed explanation
v1 -> v2 :
- Fix the reproduce commands
- Mov out the 'tnl_params' assignment line to the more suitable place
with Eric's suggestion
https://lore.kernel.org/netdev/CANn89iJoMcxe6xAOE=QGfqmOa1p+_ssSr_2y4KUJr-Qap3xk0Q@mail.gmail.com/
---
 net/ipv4/ip_gre.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 5f6fd382af38..f1f31ebfc793 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -662,11 +662,11 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
 		if (skb_cow_head(skb, 0))
 			goto free_skb;
 
-		tnl_params = (const struct iphdr *)skb->data;
-
-		if (!pskb_network_may_pull(skb, pull_len))
+		if (!pskb_may_pull(skb, pull_len))
 			goto free_skb;
 
+		tnl_params = (const struct iphdr *)skb->data;
+
 		/* ip_tunnel_xmit() needs skb->data pointing to gre header. */
 		skb_pull(skb, pull_len);
 		skb_reset_mac_header(skb);
-- 
2.39.2

On 9/25/24 01:51, Anton Danilov wrote:
> Regression Description:
> 
> Depending on the options specified for the GRE tunnel device, small
> packets may be dropped. This occurs because the pskb_network_may_pull
> function fails due to the packet's insufficient length.
> 
> For example, if only the okey option is specified for the tunnel device,
> original (before encapsulation) packets smaller than 28 bytes (including
> the IPv4 header) will be dropped. This happens because the required
> length is calculated relative to the network header, not the skb->head.
> 
> Here is how the required length is computed and checked:
> 
> * The pull_len variable is set to 28 bytes, consisting of:
>    * IPv4 header: 20 bytes
>    * GRE header with Key field: 8 bytes
> 
> * The pskb_network_may_pull function adds the network offset, shifting
> the checkable space further to the beginning of the network header and
> extending it to the beginning of the packet. As a result, the end of
> the checkable space occurs beyond the actual end of the packet.
> 
> Instead of ensuring that 28 bytes are present in skb->head, the function
> is requesting these 28 bytes starting from the network header. For small
> packets, this requested length exceeds the actual packet size, causing
> the check to fail and the packets to be dropped.
> 
> This issue affects both locally originated and forwarded packets in
> DMVPN-like setups.
> 
> How to reproduce (for local originated packets):
> 
>    ip link add dev gre1 type gre ikey 1.9.8.4 okey 1.9.8.4 \
>            local <your-ip> remote 0.0.0.0
> 
>    ip link set mtu 1400 dev gre1
>    ip link set up dev gre1
>    ip address add 192.168.13.1/24 dev gre1
>    ip neighbor add 192.168.13.2 lladdr <remote-ip> dev gre1
>    ping -s 1374 -c 10 192.168.13.2
>    tcpdump -vni gre1
>    tcpdump -vni <your-ext-iface> 'ip proto 47'
>    ip -s -s -d link show dev gre1
> 
> Solution:
> 
> Use the pskb_may_pull function instead the pskb_network_may_pull.
> 
> Fixes: 80d875cfc9d3 ("ipv4: ip_gre: Avoid skb_pull() failure in ipgre_xmit()")
> 
> Signed-off-by: Anton Danilov <littlesmilingcloud@gmail.com>

For future submissions, please note that there should be no empty line 
in the tag area - i.e. no empty line between 'Fixes' and SoB.

Thanks,

Paolo

On Wed, Sep 25, 2024 at 1:53 AM Anton Danilov
<littlesmilingcloud@gmail.com> wrote:
>
> Regression Description:
>
> Depending on the options specified for the GRE tunnel device, small
> packets may be dropped. This occurs because the pskb_network_may_pull
> function fails due to the packet's insufficient length.
>
> For example, if only the okey option is specified for the tunnel device,
> original (before encapsulation) packets smaller than 28 bytes (including
> the IPv4 header) will be dropped. This happens because the required
> length is calculated relative to the network header, not the skb->head.
>
> Here is how the required length is computed and checked:
>
> * The pull_len variable is set to 28 bytes, consisting of:
>   * IPv4 header: 20 bytes
>   * GRE header with Key field: 8 bytes
>
> * The pskb_network_may_pull function adds the network offset, shifting
> the checkable space further to the beginning of the network header and
> extending it to the beginning of the packet. As a result, the end of
> the checkable space occurs beyond the actual end of the packet.
>
> Instead of ensuring that 28 bytes are present in skb->head, the function
> is requesting these 28 bytes starting from the network header. For small
> packets, this requested length exceeds the actual packet size, causing
> the check to fail and the packets to be dropped.
>
> This issue affects both locally originated and forwarded packets in
> DMVPN-like setups.
>
> How to reproduce (for local originated packets):
>
>   ip link add dev gre1 type gre ikey 1.9.8.4 okey 1.9.8.4 \
>           local <your-ip> remote 0.0.0.0
>
>   ip link set mtu 1400 dev gre1
>   ip link set up dev gre1
>   ip address add 192.168.13.1/24 dev gre1
>   ip neighbor add 192.168.13.2 lladdr <remote-ip> dev gre1
>   ping -s 1374 -c 10 192.168.13.2
>   tcpdump -vni gre1
>   tcpdump -vni <your-ext-iface> 'ip proto 47'
>   ip -s -s -d link show dev gre1
>
> Solution:
>
> Use the pskb_may_pull function instead the pskb_network_may_pull.
>
> Fixes: 80d875cfc9d3 ("ipv4: ip_gre: Avoid skb_pull() failure in ipgre_xmit()")
>
> Signed-off-by: Anton Danilov <littlesmilingcloud@gmail.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>