Messages in this thread Patch in this message | | | Date | Wed, 29 Nov 2023 10:50:46 +0900 (JST) | Subject | Re: [PATCH net] ipv4: ip_gre: Handle skb_pull() failure in ipgre_xmit() | From | Shigeru Yoshida <> |
| |
On Mon, 27 Nov 2023 10:55:01 -0500, Willem de Bruijn wrote: > Shigeru Yoshida wrote: >> In ipgre_xmit(), skb_pull() may fail even if pskb_inet_may_pull() returns >> true. For example, applications can create a malformed packet that causes >> this problem with PF_PACKET. > > It may fail because because pskb_inet_may_pull does not account for > tunnel->hlen. > > Is that what you are referring to with malformed packet? Can you > eloborate a bit on in which way the packet has to be malformed to > reach this?
Thank you very much for your prompt feedback.
Actually, I found this problem by running syzkaller. Syzkaller reported the following uninit-value issue (I think the root cause of this issue is the same as the one Eric mentioned):
===================================================== BUG: KMSAN: uninit-value in __gre_xmit net/ipv4/ip_gre.c:469 [inline] BUG: KMSAN: uninit-value in ipgre_xmit+0xdf4/0xe70 net/ipv4/ip_gre.c:662 __gre_xmit net/ipv4/ip_gre.c:469 [inline] ipgre_xmit+0xdf4/0xe70 net/ipv4/ip_gre.c:662 __netdev_start_xmit include/linux/netdevice.h:4918 [inline] netdev_start_xmit include/linux/netdevice.h:4932 [inline] xmit_one net/core/dev.c:3543 [inline] dev_hard_start_xmit+0x24a/0xa10 net/core/dev.c:3559 __dev_queue_xmit+0x32f6/0x50e0 net/core/dev.c:4344 dev_queue_xmit include/linux/netdevice.h:3112 [inline] packet_xmit+0x8f/0x6b0 net/packet/af_packet.c:276 packet_snd net/packet/af_packet.c:3087 [inline] packet_sendmsg+0x8c24/0x9aa0 net/packet/af_packet.c:3119 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg net/socket.c:745 [inline] __sys_sendto+0x717/0xa00 net/socket.c:2194 __do_sys_sendto net/socket.c:2206 [inline] __se_sys_sendto net/socket.c:2202 [inline] __x64_sys_sendto+0x130/0x200 net/socket.c:2202 do_syscall_x64 arch/x86/entry/common.c:51 [inline] do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82 entry_SYSCALL_64_after_hwframe+0x63/0x6b
Uninit was stored to memory at: __gre_xmit net/ipv4/ip_gre.c:469 [inline] ipgre_xmit+0xded/0xe70 net/ipv4/ip_gre.c:662 __netdev_start_xmit include/linux/netdevice.h:4918 [inline] netdev_start_xmit include/linux/netdevice.h:4932 [inline] xmit_one net/core/dev.c:3543 [inline] dev_hard_start_xmit+0x24a/0xa10 net/core/dev.c:3559 __dev_queue_xmit+0x32f6/0x50e0 net/core/dev.c:4344 dev_queue_xmit include/linux/netdevice.h:3112 [inline] packet_xmit+0x8f/0x6b0 net/packet/af_packet.c:276 packet_snd net/packet/af_packet.c:3087 [inline] packet_sendmsg+0x8c24/0x9aa0 net/packet/af_packet.c:3119 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg net/socket.c:745 [inline] __sys_sendto+0x717/0xa00 net/socket.c:2194 __do_sys_sendto net/socket.c:2206 [inline] __se_sys_sendto net/socket.c:2202 [inline] __x64_sys_sendto+0x130/0x200 net/socket.c:2202 do_syscall_x64 arch/x86/entry/common.c:51 [inline] do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82 entry_SYSCALL_64_after_hwframe+0x63/0x6b
Uninit was created at: slab_post_alloc_hook+0x103/0x9e0 mm/slab.h:768 slab_alloc_node mm/slub.c:3478 [inline] kmem_cache_alloc_node+0x5f7/0xb50 mm/slub.c:3523 kmalloc_reserve+0x13c/0x4a0 net/core/skbuff.c:560 pskb_expand_head+0x20b/0x19a0 net/core/skbuff.c:2098 __skb_cow include/linux/skbuff.h:3586 [inline] skb_cow_head include/linux/skbuff.h:3620 [inline] ipgre_xmit+0x73c/0xe70 net/ipv4/ip_gre.c:638 __netdev_start_xmit include/linux/netdevice.h:4918 [inline] netdev_start_xmit include/linux/netdevice.h:4932 [inline] xmit_one net/core/dev.c:3543 [inline] dev_hard_start_xmit+0x24a/0xa10 net/core/dev.c:3559 __dev_queue_xmit+0x32f6/0x50e0 net/core/dev.c:4344 dev_queue_xmit include/linux/netdevice.h:3112 [inline] packet_xmit+0x8f/0x6b0 net/packet/af_packet.c:276 packet_snd net/packet/af_packet.c:3087 [inline] packet_sendmsg+0x8c24/0x9aa0 net/packet/af_packet.c:3119 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg net/socket.c:745 [inline] __sys_sendto+0x717/0xa00 net/socket.c:2194 __do_sys_sendto net/socket.c:2206 [inline] __se_sys_sendto net/socket.c:2202 [inline] __x64_sys_sendto+0x130/0x200 net/socket.c:2202 do_syscall_x64 arch/x86/entry/common.c:51 [inline] do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82 entry_SYSCALL_64_after_hwframe+0x63/0x6b
CPU: 1 PID: 11318 Comm: syz-executor.7 Not tainted 6.6.0-14500-g1c41041124bd #10 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc38 04/01/2014 =====================================================
The simplified version of the repro is shown below:
#include <linux/if_ether.h> #include <sys/ioctl.h> #include <netinet/ether.h> #include <net/if.h> #include <sys/socket.h> #include <netinet/in.h> #include <string.h> #include <unistd.h> #include <stdio.h> #include <stdlib.h> #include <linux/if_packet.h>
int main(void) { int s, s1, s2, data = 0; struct ifreq ifr; struct sockaddr_ll addr = { 0 }; unsigned char mac_addr[] = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6};
s = socket(AF_PACKET, SOCK_DGRAM, 0x300); s1 = socket(AF_PACKET, SOCK_RAW, 0x300); s2 = socket(AF_NETLINK, SOCK_RAW, 0);
strcpy(ifr.ifr_name, "gre0"); ioctl(s2, SIOCGIFINDEX, &ifr);
addr.sll_family = AF_PACKET; addr.sll_ifindex = ifr.ifr_ifindex; addr.sll_protocol = htons(0); addr.sll_hatype = ARPHRD_ETHER; addr.sll_pkttype = PACKET_HOST; addr.sll_halen = ETH_ALEN; memcpy(addr.sll_addr, mac_addr, ETH_ALEN);
sendto(s1, &data, 1, 0, (struct sockaddr *)&addr, sizeof(addr));
return 0; }
The repro sends a 1-byte packet that doesn't have the correct IP header. I meant this as "malformed pachet", but that might be a bit confusing, sorry.
I think the cause of the uninit-value access is that ipgre_xmit() reallocates the skb with skb_cow_head() and copies only the 1-byte data, so any IP header access through `tnl_params` can cause the problem.
At first I tried to modify pskb_inet_may_pull() to detect this type of packet, but I ended up doing this patch.
Any advice is welcome!
Thanks, Shigeru
> > FYI: I had a quick look at the IPv6 equivalent code. > ip6gre_tunnel_xmit is sufficiently different. It makes sense that this > is an IPv4 only patch. > >> This patch fixes the problem by dropping skb and returning from the >> function if skb_pull() fails. >> >> Fixes: c54419321455 ("GRE: Refactor GRE tunneling code.") >> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com> >> --- >> net/ipv4/ip_gre.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c >> index 22a26d1d29a0..95efa97cb84b 100644 >> --- a/net/ipv4/ip_gre.c >> +++ b/net/ipv4/ip_gre.c >> @@ -643,7 +643,8 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb, >> /* Pull skb since ip_tunnel_xmit() needs skb->data pointing >> * to gre header. >> */ >> - skb_pull(skb, tunnel->hlen + sizeof(struct iphdr)); >> + if (!skb_pull(skb, tunnel->hlen + sizeof(struct iphdr))) >> + goto free_skb; >> skb_reset_mac_header(skb); >> >> if (skb->ip_summed == CHECKSUM_PARTIAL && >> -- >> 2.41.0 >> > >
| |