From: Eric Dumazet <edumazet@google.com>
To: Shigeru Yoshida <syoshida@redhat.com>
Cc: davem@davemloft.net, dsahern@kernel.org, kuba@kernel.org,
pabeni@redhat.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
syzkaller <syzkaller@googlegroups.com>
Subject: Re: [PATCH net] ipv4: Fix uninit-value access in __ip_make_skb()
Date: Mon, 25 Mar 2024 10:01:25 +0100 [thread overview]
Message-ID: <CANn89iL_Oz58VYNLJ6eB=qgmsgY9juo9xAhaPKKaDqOxrjf+0w@mail.gmail.com> (raw)
In-Reply-To: <20240324050554.1609460-1-syoshida@redhat.com>
On Sun, Mar 24, 2024 at 6:06 AM Shigeru Yoshida <syoshida@redhat.com> 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.
>
> [1]
What is the kernel version for this trace ?
> 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
>
> Fixes: 99e5acae193e ("ipv4: Fix potential uninit variable access bug in __ip_make_skb()")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
> ---
> I think IPv6 has a similar issue. If this patch is accepted, I will send
> a patch for IPv6.
> ---
> net/ipv4/ip_output.c | 2 +-
> 1 file changed, 1 insertion(+), 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;
> --
> 2.44.0
>
Thanks for your patch.
I do not think this is enough, as far as syzkaller is concerned.
raw_probe_proto_opt() can leave garbage in fl4_icmp_type (and fl4_icmp_code)
next prev parent reply other threads:[~2024-03-25 9:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-24 5:05 [PATCH net] ipv4: Fix uninit-value access in __ip_make_skb() Shigeru Yoshida
2024-03-25 9:01 ` Eric Dumazet [this message]
2024-03-25 9:38 ` Shigeru Yoshida
2024-03-25 10:05 ` Eric Dumazet
2024-03-25 17:46 ` Shigeru Yoshida
2024-03-25 19:56 ` Eric Dumazet
2024-03-29 8:02 ` Shigeru Yoshida
-- strict thread matches above, loose matches on Subject: below --
2024-04-29 15:16 Shigeru Yoshida
2024-04-30 12:38 ` Shigeru Yoshida
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANn89iL_Oz58VYNLJ6eB=qgmsgY9juo9xAhaPKKaDqOxrjf+0w@mail.gmail.com' \
--to=edumazet@google.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=syoshida@redhat.com \
--cc=syzkaller@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.