KMSAN reported uninit-value access in __ip_make_skb() [1]. __ip_make_skb()
tests HDRINCL to know if the skb has icmphdr. However, HDRINCL can cause a
race condition. If calling setsockopt(2) with IP_HDRINCL changes HDRINCL
while __ip_make_skb() is running, the function will access icmphdr in the
skb even if it is not included. This causes the issue reported by KMSAN.
Check FLOWI_FLAG_KNOWN_NH on fl4->flowi4_flags instead of testing HDRINCL
on the socket.
Also, fl4->fl4_icmp_type and fl4->fl4_icmp_code are not initialized. These
are union in struct flowi4 and are implicitly initialized by
flowi4_init_output(), but we should not rely on specific union layout.
Initialize these explicitly in raw_sendmsg().
[1]
BUG: KMSAN: uninit-value in __ip_make_skb+0x2b74/0x2d20 net/ipv4/ip_output.c:1481
__ip_make_skb+0x2b74/0x2d20 net/ipv4/ip_output.c:1481
ip_finish_skb include/net/ip.h:243 [inline]
ip_push_pending_frames+0x4c/0x5c0 net/ipv4/ip_output.c:1508
raw_sendmsg+0x2381/0x2690 net/ipv4/raw.c:654
inet_sendmsg+0x27b/0x2a0 net/ipv4/af_inet.c:851
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x274/0x3c0 net/socket.c:745
__sys_sendto+0x62c/0x7b0 net/socket.c:2191
__do_sys_sendto net/socket.c:2203 [inline]
__se_sys_sendto net/socket.c:2199 [inline]
__x64_sys_sendto+0x130/0x200 net/socket.c:2199
do_syscall_64+0xd8/0x1f0 arch/x86/entry/common.c:83
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+0x5f6/0xc50 mm/slub.c:3888
kmalloc_reserve+0x13c/0x4a0 net/core/skbuff.c:577
__alloc_skb+0x35a/0x7c0 net/core/skbuff.c:668
alloc_skb include/linux/skbuff.h:1318 [inline]
__ip_append_data+0x49ab/0x68c0 net/ipv4/ip_output.c:1128
ip_append_data+0x1e7/0x260 net/ipv4/ip_output.c:1365
raw_sendmsg+0x22b1/0x2690 net/ipv4/raw.c:648
inet_sendmsg+0x27b/0x2a0 net/ipv4/af_inet.c:851
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x274/0x3c0 net/socket.c:745
__sys_sendto+0x62c/0x7b0 net/socket.c:2191
__do_sys_sendto net/socket.c:2203 [inline]
__se_sys_sendto net/socket.c:2199 [inline]
__x64_sys_sendto+0x130/0x200 net/socket.c:2199
do_syscall_64+0xd8/0x1f0 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x6d/0x75
CPU: 1 PID: 15709 Comm: syz-executor.7 Not tainted 6.8.0-11567-gb3603fcb79b1 #25
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
Fixes: 99e5acae193e ("ipv4: Fix potential uninit variable access bug in __ip_make_skb()")
Reported-by: syzkaller <[email protected]>
Signed-off-by: Shigeru Yoshida <[email protected]>
---
v1->v2: https://lore.kernel.org/all/[email protected]/
- Explicitly initialize fl4->fl4_icmp_type and fl4->fl4_icmp_code because
we should not rely on a specific union layout.
---
net/ipv4/ip_output.c | 2 +-
net/ipv4/raw.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 1fe794967211..39229fd0601a 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1473,7 +1473,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
* by icmp_hdr(skb)->type.
*/
if (sk->sk_type == SOCK_RAW &&
- !inet_test_bit(HDRINCL, sk))
+ !(fl4->flowi4_flags & FLOWI_FLAG_KNOWN_NH))
icmp_type = fl4->fl4_icmp_type;
else
icmp_type = icmp_hdr(skb)->type;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index dcb11f22cbf2..4cb43401e0e0 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -612,6 +612,9 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
(hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
daddr, saddr, 0, 0, sk->sk_uid);
+ fl4.fl4_icmp_type = 0;
+ fl4.fl4_icmp_code = 0;
+
if (!hdrincl) {
rfv.msg = msg;
rfv.hlen = 0;
--
2.44.0
Sorry, I forgot to add "v2" to the prefix. I will repost this.
On Tue, 30 Apr 2024 00:16:05 +0900, Shigeru Yoshida wrote:
> KMSAN reported uninit-value access in __ip_make_skb() [1]. __ip_make_skb()
> tests HDRINCL to know if the skb has icmphdr. However, HDRINCL can cause a
> race condition. If calling setsockopt(2) with IP_HDRINCL changes HDRINCL
> while __ip_make_skb() is running, the function will access icmphdr in the
> skb even if it is not included. This causes the issue reported by KMSAN.
>
> Check FLOWI_FLAG_KNOWN_NH on fl4->flowi4_flags instead of testing HDRINCL
> on the socket.
>
> Also, fl4->fl4_icmp_type and fl4->fl4_icmp_code are not initialized. These
> are union in struct flowi4 and are implicitly initialized by
> flowi4_init_output(), but we should not rely on specific union layout.
>
> Initialize these explicitly in raw_sendmsg().
>
> [1]
> BUG: KMSAN: uninit-value in __ip_make_skb+0x2b74/0x2d20 net/ipv4/ip_output.c:1481
> __ip_make_skb+0x2b74/0x2d20 net/ipv4/ip_output.c:1481
> ip_finish_skb include/net/ip.h:243 [inline]
> ip_push_pending_frames+0x4c/0x5c0 net/ipv4/ip_output.c:1508
> raw_sendmsg+0x2381/0x2690 net/ipv4/raw.c:654
> inet_sendmsg+0x27b/0x2a0 net/ipv4/af_inet.c:851
> sock_sendmsg_nosec net/socket.c:730 [inline]
> __sock_sendmsg+0x274/0x3c0 net/socket.c:745
> __sys_sendto+0x62c/0x7b0 net/socket.c:2191
> __do_sys_sendto net/socket.c:2203 [inline]
> __se_sys_sendto net/socket.c:2199 [inline]
> __x64_sys_sendto+0x130/0x200 net/socket.c:2199
> do_syscall_64+0xd8/0x1f0 arch/x86/entry/common.c:83
> 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+0x5f6/0xc50 mm/slub.c:3888
> kmalloc_reserve+0x13c/0x4a0 net/core/skbuff.c:577
> __alloc_skb+0x35a/0x7c0 net/core/skbuff.c:668
> alloc_skb include/linux/skbuff.h:1318 [inline]
> __ip_append_data+0x49ab/0x68c0 net/ipv4/ip_output.c:1128
> ip_append_data+0x1e7/0x260 net/ipv4/ip_output.c:1365
> raw_sendmsg+0x22b1/0x2690 net/ipv4/raw.c:648
> inet_sendmsg+0x27b/0x2a0 net/ipv4/af_inet.c:851
> sock_sendmsg_nosec net/socket.c:730 [inline]
> __sock_sendmsg+0x274/0x3c0 net/socket.c:745
> __sys_sendto+0x62c/0x7b0 net/socket.c:2191
> __do_sys_sendto net/socket.c:2203 [inline]
> __se_sys_sendto net/socket.c:2199 [inline]
> __x64_sys_sendto+0x130/0x200 net/socket.c:2199
> do_syscall_64+0xd8/0x1f0 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x6d/0x75
>
> CPU: 1 PID: 15709 Comm: syz-executor.7 Not tainted 6.8.0-11567-gb3603fcb79b1 #25
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
>
> Fixes: 99e5acae193e ("ipv4: Fix potential uninit variable access bug in __ip_make_skb()")
> Reported-by: syzkaller <[email protected]>
> Signed-off-by: Shigeru Yoshida <[email protected]>
> ---
> v1->v2: https://lore.kernel.org/all/[email protected]/
> - Explicitly initialize fl4->fl4_icmp_type and fl4->fl4_icmp_code because
> we should not rely on a specific union layout.
> ---
> net/ipv4/ip_output.c | 2 +-
> net/ipv4/raw.c | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 1fe794967211..39229fd0601a 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1473,7 +1473,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
> * by icmp_hdr(skb)->type.
> */
> if (sk->sk_type == SOCK_RAW &&
> - !inet_test_bit(HDRINCL, sk))
> + !(fl4->flowi4_flags & FLOWI_FLAG_KNOWN_NH))
> icmp_type = fl4->fl4_icmp_type;
> else
> icmp_type = icmp_hdr(skb)->type;
> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> index dcb11f22cbf2..4cb43401e0e0 100644
> --- a/net/ipv4/raw.c
> +++ b/net/ipv4/raw.c
> @@ -612,6 +612,9 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> (hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
> daddr, saddr, 0, 0, sk->sk_uid);
>
> + fl4.fl4_icmp_type = 0;
> + fl4.fl4_icmp_code = 0;
> +
> if (!hdrincl) {
> rfv.msg = msg;
> rfv.hlen = 0;
> --
> 2.44.0
>