From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Eric Dumazet <edumazet@google.com>,
syzbot <syzkaller@googlegroups.com>,
"David S . Miller" <davem@davemloft.net>,
Sasha Levin <sashal@kernel.org>,
netdev@vger.kernel.org
Subject: [PATCH AUTOSEL 4.19 08/16] net/packet: annotate accesses to po->bind
Date: Mon, 21 Jun 2021 13:54:42 -0400 [thread overview]
Message-ID: <20210621175450.736067-8-sashal@kernel.org> (raw)
In-Reply-To: <20210621175450.736067-1-sashal@kernel.org>
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit c7d2ef5dd4b03ed0ee1d13bc0c55f9cf62d49bd6 ]
tpacket_snd(), packet_snd(), packet_getname() and packet_seq_show()
can read po->num without holding a lock. This means other threads
can change po->num at the same time.
KCSAN complained about this known fact [1]
Add READ_ONCE()/WRITE_ONCE() to address the issue.
[1] BUG: KCSAN: data-race in packet_do_bind / packet_sendmsg
write to 0xffff888131a0dcc0 of 2 bytes by task 24714 on cpu 0:
packet_do_bind+0x3ab/0x7e0 net/packet/af_packet.c:3181
packet_bind+0xc3/0xd0 net/packet/af_packet.c:3255
__sys_bind+0x200/0x290 net/socket.c:1637
__do_sys_bind net/socket.c:1648 [inline]
__se_sys_bind net/socket.c:1646 [inline]
__x64_sys_bind+0x3d/0x50 net/socket.c:1646
do_syscall_64+0x4a/0x90 arch/x86/entry/common.c:47
entry_SYSCALL_64_after_hwframe+0x44/0xae
read to 0xffff888131a0dcc0 of 2 bytes by task 24719 on cpu 1:
packet_snd net/packet/af_packet.c:2899 [inline]
packet_sendmsg+0x317/0x3570 net/packet/af_packet.c:3040
sock_sendmsg_nosec net/socket.c:654 [inline]
sock_sendmsg net/socket.c:674 [inline]
____sys_sendmsg+0x360/0x4d0 net/socket.c:2350
___sys_sendmsg net/socket.c:2404 [inline]
__sys_sendmsg+0x1ed/0x270 net/socket.c:2433
__do_sys_sendmsg net/socket.c:2442 [inline]
__se_sys_sendmsg net/socket.c:2440 [inline]
__x64_sys_sendmsg+0x42/0x50 net/socket.c:2440
do_syscall_64+0x4a/0x90 arch/x86/entry/common.c:47
entry_SYSCALL_64_after_hwframe+0x44/0xae
value changed: 0x0000 -> 0x1200
Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 24719 Comm: syz-executor.5 Not tainted 5.13.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/packet/af_packet.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 16b745d254fe..42299fa02038 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2656,7 +2656,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
}
if (likely(saddr == NULL)) {
dev = packet_cached_dev_get(po);
- proto = po->num;
+ proto = READ_ONCE(po->num);
} else {
err = -EINVAL;
if (msg->msg_namelen < sizeof(struct sockaddr_ll))
@@ -2869,7 +2869,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
if (likely(saddr == NULL)) {
dev = packet_cached_dev_get(po);
- proto = po->num;
+ proto = READ_ONCE(po->num);
} else {
err = -EINVAL;
if (msg->msg_namelen < sizeof(struct sockaddr_ll))
@@ -3141,7 +3141,7 @@ static int packet_do_bind(struct sock *sk, const char *name, int ifindex,
/* prevents packet_notifier() from calling
* register_prot_hook()
*/
- po->num = 0;
+ WRITE_ONCE(po->num, 0);
__unregister_prot_hook(sk, true);
rcu_read_lock();
dev_curr = po->prot_hook.dev;
@@ -3151,7 +3151,7 @@ static int packet_do_bind(struct sock *sk, const char *name, int ifindex,
}
BUG_ON(po->running);
- po->num = proto;
+ WRITE_ONCE(po->num, proto);
po->prot_hook.type = proto;
if (unlikely(unlisted)) {
@@ -3497,7 +3497,7 @@ static int packet_getname(struct socket *sock, struct sockaddr *uaddr,
sll->sll_family = AF_PACKET;
sll->sll_ifindex = po->ifindex;
- sll->sll_protocol = po->num;
+ sll->sll_protocol = READ_ONCE(po->num);
sll->sll_pkttype = 0;
rcu_read_lock();
dev = dev_get_by_index_rcu(sock_net(sk), po->ifindex);
@@ -4391,7 +4391,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
was_running = po->running;
num = po->num;
if (was_running) {
- po->num = 0;
+ WRITE_ONCE(po->num, 0);
__unregister_prot_hook(sk, false);
}
spin_unlock(&po->bind_lock);
@@ -4426,7 +4426,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
spin_lock(&po->bind_lock);
if (was_running) {
- po->num = num;
+ WRITE_ONCE(po->num, num);
register_prot_hook(sk);
}
spin_unlock(&po->bind_lock);
@@ -4597,7 +4597,7 @@ static int packet_seq_show(struct seq_file *seq, void *v)
s,
refcount_read(&s->sk_refcnt),
s->sk_type,
- ntohs(po->num),
+ ntohs(READ_ONCE(po->num)),
po->ifindex,
po->running,
atomic_read(&s->sk_rmem_alloc),
--
2.30.2
next prev parent reply other threads:[~2021-06-21 18:04 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-21 17:54 [PATCH AUTOSEL 4.19 01/16] mac80211: remove warning in ieee80211_get_sband() Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 02/16] cfg80211: call cfg80211_leave_ocb when switching away from OCB Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 03/16] mac80211: drop multicast fragments Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 04/16] net: ethtool: clear heap allocations for ethtool function Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 05/16] ping: Check return value of function 'ping_queue_rcv_skb' Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 06/16] inet: annotate date races around sk->sk_txhash Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 07/16] net: caif: fix memory leak in ldisc_open Sasha Levin
2021-06-21 17:54 ` Sasha Levin [this message]
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 09/16] net/packet: annotate accesses to po->ifindex Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 10/16] r8152: Avoid memcpy() over-reading of ETH_SS_STATS Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 11/16] sh_eth: " Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 12/16] r8169: " Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 13/16] KVM: selftests: Fix kvm_check_cap() assertion Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 14/16] net: qed: Fix memcpy() overflow of qed_dcbx_params() Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 15/16] PCI: Add AMD RS690 quirk to enable 64-bit DMA Sasha Levin
2021-06-21 17:54 ` [PATCH AUTOSEL 4.19 16/16] net: ll_temac: Avoid ndo_start_xmit returning NETDEV_TX_BUSY Sasha Levin
2021-06-21 17:54 ` Sasha Levin
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=20210621175450.736067-8-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=stable@vger.kernel.org \
--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.