netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: <kuniyu@amazon.com>
Cc: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
	<kuni1840@gmail.com>, <netdev@vger.kernel.org>,
	<pabeni@redhat.com>, <syzkaller@googlegroups.com>
Subject: Re: [PATCH v1 net] net: Fix sk->sk_stamp race in sock_recv_cmsgs().
Date: Fri, 5 May 2023 20:01:12 -0700	[thread overview]
Message-ID: <20230506030113.6853-1-kuniyu@amazon.com> (raw)
In-Reply-To: <20230506022325.99106-1-kuniyu@amazon.com>

From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Fri, 5 May 2023 19:23:25 -0700
> KCSAN found a data race in sock_recv_cmsgs() [0] where the read access
> to sk->sk_stamp needs READ_ONCE().
> 
> Also, there is another race below.  If the torn load of the high 32-bits
> precedes WRITE_ONCE(sk, skb->tstamp) and later the written lower 32-bits
> happens to match with SK_DEFAULT_STAMP, the final result of sk->sk_stamp
> could be 0.
> 
>   sock_recv_cmsgs()  ioctl(SIOCGSTAMP)      sock_recv_cmsgs()
>   |                  |                      |
>   |- if (sock_flag(sk, SOCK_TIMESTAMP))     |
>   |                  |                      |
>   |                  `- sock_set_flag(sk, SOCK_TIMESTAMP)
>   |                                         |
>   |                                          `- if (sock_flag(sk, SOCK_TIMESTAMP))
>   `- if (sk->sk_stamp == SK_DEFAULT_STAMP)      `- sock_write_timestamp(sk, skb->tstamp)
>       `- sock_write_timestamp(sk, 0)
> 
> Even with READ_ONCE(), we could get the same result if READ_ONCE() precedes
> WRITE_ONCE() because the SK_DEFAULT_STAMP check and WRITE_ONCE(sk_stamp, 0)
> are not atomic.
> 
> Let's avoid the race by cmpxchg() on 64-bits architecture or seqlock on
> 32-bits machines.
> 
> [0]:
> BUG: KCSAN: data-race in packet_recvmsg / packet_recvmsg
> 
> write (marked) to 0xffff88803c81f258 of 8 bytes by task 19171 on cpu 0:
>  sock_write_timestamp include/net/sock.h:2670 [inline]
>  sock_recv_cmsgs include/net/sock.h:2722 [inline]
>  packet_recvmsg+0xb97/0xd00 net/packet/af_packet.c:3489
>  sock_recvmsg_nosec net/socket.c:1019 [inline]
>  sock_recvmsg+0x11a/0x130 net/socket.c:1040
>  sock_read_iter+0x176/0x220 net/socket.c:1118
>  call_read_iter include/linux/fs.h:1845 [inline]
>  new_sync_read fs/read_write.c:389 [inline]
>  vfs_read+0x5e0/0x630 fs/read_write.c:470
>  ksys_read+0x163/0x1a0 fs/read_write.c:613
>  __do_sys_read fs/read_write.c:623 [inline]
>  __se_sys_read fs/read_write.c:621 [inline]
>  __x64_sys_read+0x41/0x50 fs/read_write.c:621
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> 
> read to 0xffff88803c81f258 of 8 bytes by task 19183 on cpu 1:
>  sock_recv_cmsgs include/net/sock.h:2721 [inline]
>  packet_recvmsg+0xb64/0xd00 net/packet/af_packet.c:3489
>  sock_recvmsg_nosec net/socket.c:1019 [inline]
>  sock_recvmsg+0x11a/0x130 net/socket.c:1040
>  sock_read_iter+0x176/0x220 net/socket.c:1118
>  call_read_iter include/linux/fs.h:1845 [inline]
>  new_sync_read fs/read_write.c:389 [inline]
>  vfs_read+0x5e0/0x630 fs/read_write.c:470
>  ksys_read+0x163/0x1a0 fs/read_write.c:613
>  __do_sys_read fs/read_write.c:623 [inline]
>  __se_sys_read fs/read_write.c:621 [inline]
>  __x64_sys_read+0x41/0x50 fs/read_write.c:621
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> 
> value changed: 0xffffffffc4653600 -> 0x0000000000000000
> 
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 1 PID: 19183 Comm: syz-executor.5 Not tainted 6.3.0-rc7-02330-gca6270c12e20 #2
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
>

Sorry, here should be:

Fixes: 6c7c98bad488 ("sock: avoid dirtying sk_stamp, if possible")

> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

      reply	other threads:[~2023-05-06  3:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-06  2:23 [PATCH v1 net] net: Fix sk->sk_stamp race in sock_recv_cmsgs() Kuniyuki Iwashima
2023-05-06  3:01 ` Kuniyuki Iwashima [this message]

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=20230506030113.6853-1-kuniyu@amazon.com \
    --to=kuniyu@amazon.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=kuni1840@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).