Skip to content

Commit eae2d23

Browse files
committed
geneve: fix header validation in geneve[6]_xmit_skb
jira LE-1733 bugfix-pre geneve_fixes commit d8a6213 syzbot is able to trigger an uninit-value in geneve_xmit() [1] Problem : While most ip tunnel helpers (like ip_tunnel_get_dsfield()) uses skb_protocol(skb, true), pskb_inet_may_pull() is only using skb->protocol. If anything else than ETH_P_IPV6 or ETH_P_IP is found in skb->protocol, pskb_inet_may_pull() does nothing at all. If a vlan tag was provided by the caller (af_packet in the syzbot case), the network header might not point to the correct location, and skb linear part could be smaller than expected. Add skb_vlan_inet_prepare() to perform a complete mac validation. Use this in geneve for the moment, I suspect we need to adopt this more broadly. v4 - Jakub reported v3 broke l2_tos_ttl_inherit.sh selftest - Only call __vlan_get_protocol() for vlan types. Link: https://lore.kernel.org/netdev/20240404100035.3270a7d5@kernel.org/ v2,v3 - Addressed Sabrina comments on v1 and v2 Link: https://lore.kernel.org/netdev/Zg1l9L2BNoZWZDZG@hog/ [1] BUG: KMSAN: uninit-value in geneve_xmit_skb drivers/net/geneve.c:910 [inline] BUG: KMSAN: uninit-value in geneve_xmit+0x302d/0x5420 drivers/net/geneve.c:1030 geneve_xmit_skb drivers/net/geneve.c:910 [inline] geneve_xmit+0x302d/0x5420 drivers/net/geneve.c:1030 __netdev_start_xmit include/linux/netdevice.h:4903 [inline] netdev_start_xmit include/linux/netdevice.h:4917 [inline] xmit_one net/core/dev.c:3531 [inline] dev_hard_start_xmit+0x247/0xa20 net/core/dev.c:3547 __dev_queue_xmit+0x348d/0x52c0 net/core/dev.c:4335 dev_queue_xmit include/linux/netdevice.h:3091 [inline] packet_xmit+0x9c/0x6c0 net/packet/af_packet.c:276 packet_snd net/packet/af_packet.c:3081 [inline] packet_sendmsg+0x8bb0/0x9ef0 net/packet/af_packet.c:3113 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg+0x30f/0x380 net/socket.c:745 __sys_sendto+0x685/0x830 net/socket.c:2191 __do_sys_sendto net/socket.c:2203 [inline] __se_sys_sendto net/socket.c:2199 [inline] __x64_sys_sendto+0x125/0x1d0 net/socket.c:2199 do_syscall_64+0xd5/0x1f0 entry_SYSCALL_64_after_hwframe+0x6d/0x75 Uninit was created at: slab_post_alloc_hook mm/slub.c:3804 [inline] slab_alloc_node mm/slub.c:3845 [inline] kmem_cache_alloc_node+0x613/0xc50 mm/slub.c:3888 kmalloc_reserve+0x13d/0x4a0 net/core/skbuff.c:577 __alloc_skb+0x35b/0x7a0 net/core/skbuff.c:668 alloc_skb include/linux/skbuff.h:1318 [inline] alloc_skb_with_frags+0xc8/0xbf0 net/core/skbuff.c:6504 sock_alloc_send_pskb+0xa81/0xbf0 net/core/sock.c:2795 packet_alloc_skb net/packet/af_packet.c:2930 [inline] packet_snd net/packet/af_packet.c:3024 [inline] packet_sendmsg+0x722d/0x9ef0 net/packet/af_packet.c:3113 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg+0x30f/0x380 net/socket.c:745 __sys_sendto+0x685/0x830 net/socket.c:2191 __do_sys_sendto net/socket.c:2203 [inline] __se_sys_sendto net/socket.c:2199 [inline] __x64_sys_sendto+0x125/0x1d0 net/socket.c:2199 do_syscall_64+0xd5/0x1f0 entry_SYSCALL_64_after_hwframe+0x6d/0x75 CPU: 0 PID: 5033 Comm: syz-executor346 Not tainted 6.9.0-rc1-syzkaller-00005-g928a87efa423 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/29/2024 Fixes: d13f048 ("net: geneve: modify IP header check in geneve6_xmit_skb and geneve_xmit_skb") Reported-by: syzbot+9ee20ec1de7b3168db09@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/000000000000d19c3a06152f9ee4@google.com/ Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Phillip Potter <phil@philpotter.co.uk> Cc: Sabrina Dubroca <sd@queasysnail.net> Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> Reviewed-by: Phillip Potter <phil@philpotter.co.uk> Signed-off-by: David S. Miller <davem@davemloft.net> (cherry picked from commit d8a6213) Signed-off-by: Jonathan Maple <jmaple@ciq.com>
1 parent b801632 commit eae2d23

File tree

2 files changed

+35
-2
lines changed

2 files changed

+35
-2
lines changed

drivers/net/geneve.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
897897
__be16 sport;
898898
int err;
899899

900-
if (!pskb_inet_may_pull(skb))
900+
if (!skb_vlan_inet_prepare(skb))
901901
return -EINVAL;
902902

903903
sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
@@ -994,7 +994,7 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
994994
__be16 sport;
995995
int err;
996996

997-
if (!pskb_inet_may_pull(skb))
997+
if (!skb_vlan_inet_prepare(skb))
998998
return -EINVAL;
999999

10001000
sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);

include/net/ip_tunnels.h

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,39 @@ static inline bool pskb_inet_may_pull(struct sk_buff *skb)
345345
return pskb_network_may_pull(skb, nhlen);
346346
}
347347

348+
/* Variant of pskb_inet_may_pull().
349+
*/
350+
static inline bool skb_vlan_inet_prepare(struct sk_buff *skb)
351+
{
352+
int nhlen = 0, maclen = ETH_HLEN;
353+
__be16 type = skb->protocol;
354+
355+
/* Essentially this is skb_protocol(skb, true)
356+
* And we get MAC len.
357+
*/
358+
if (eth_type_vlan(type))
359+
type = __vlan_get_protocol(skb, type, &maclen);
360+
361+
switch (type) {
362+
#if IS_ENABLED(CONFIG_IPV6)
363+
case htons(ETH_P_IPV6):
364+
nhlen = sizeof(struct ipv6hdr);
365+
break;
366+
#endif
367+
case htons(ETH_P_IP):
368+
nhlen = sizeof(struct iphdr);
369+
break;
370+
}
371+
/* For ETH_P_IPV6/ETH_P_IP we make sure to pull
372+
* a base network header in skb->head.
373+
*/
374+
if (!pskb_may_pull(skb, maclen + nhlen))
375+
return false;
376+
377+
skb_set_network_header(skb, maclen);
378+
return true;
379+
}
380+
348381
static inline int ip_encap_hlen(struct ip_tunnel_encap *e)
349382
{
350383
const struct ip_tunnel_encap_ops *ops;

0 commit comments

Comments
 (0)