| 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
| ||
|
Message-Id: <20231129.105046.2126277148145584341.syoshida@redhat.com>
Date: Wed, 29 Nov 2023 10:50:46 +0900 (JST)
From: Shigeru Yoshida <syoshida@...hat.com>
To: willemdebruijn.kernel@...il.com, edumazet@...gle.com, pabeni@...hat.com
Cc: davem@...emloft.net, dsahern@...nel.org, kuba@...nel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] ipv4: ip_gre: Handle skb_pull() failure in
ipgre_xmit()
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@...hat.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
>>
>
>
Powered by blists - more mailing lists