* [PATCH net v2] net: Fix suspicious RCU usage in bpf_sk_reuseport_detach()
@ 2022-08-16 13:26 David Howells
2022-08-17 5:19 ` kernel test robot
2022-08-17 8:13 ` kernel test robot
0 siblings, 2 replies; 3+ messages in thread
From: David Howells @ 2022-08-16 13:26 UTC (permalink / raw)
To: yin31149; +Cc: Jakub Kicinski, netdev, dhowells, linux-kernel
bpf_sk_reuseport_detach() calls __rcu_dereference_sk_user_data_with_flags()
to obtain the value of sk->sk_user_data, but that function is only usable
if the RCU read lock is held, and neither that function nor any of its
callers hold it.
Fix this by adding a new helper,
__rcu_dereference_sk_user_data_with_flags_check() that checks to see if
sk->sk_callback_lock() is held and use that here instead.
__rcu_dereference_sk_user_data_with_flags() then calls that, supplying false
as condition indicating only the RCU read lock should be checked.
Without this, the following warning can be occasionally observed:
=============================
WARNING: suspicious RCU usage
6.0.0-rc1-build2+ #563 Not tainted
-----------------------------
include/net/sock.h:592 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
5 locks held by locktest/29873:
#0: ffff88812734b550 (&sb->s_type->i_mutex_key#9){+.+.}-{3:3}, at: __sock_release+0x77/0x121
#1: ffff88812f5621b0 (sk_lock-AF_INET){+.+.}-{0:0}, at: tcp_close+0x1c/0x70
#2: ffff88810312f5c8 (&h->lhash2[i].lock){+.+.}-{2:2}, at: inet_unhash+0x76/0x1c0
#3: ffffffff83768bb8 (reuseport_lock){+...}-{2:2}, at: reuseport_detach_sock+0x18/0xdd
#4: ffff88812f562438 (clock-AF_INET){++..}-{2:2}, at: bpf_sk_reuseport_detach+0x24/0xa4
stack backtrace:
CPU: 1 PID: 29873 Comm: locktest Not tainted 6.0.0-rc1-build2+ #563
Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014
Call Trace:
<TASK>
dump_stack_lvl+0x4c/0x5f
bpf_sk_reuseport_detach+0x6d/0xa4
reuseport_detach_sock+0x75/0xdd
inet_unhash+0xa5/0x1c0
tcp_set_state+0x169/0x20f
? lockdep_sock_is_held+0x3a/0x3a
? __lock_release.isra.0+0x13e/0x220
? reacquire_held_locks+0x1bb/0x1bb
? hlock_class+0x31/0x96
? mark_lock+0x9e/0x1af
__tcp_close+0x50/0x4b6
tcp_close+0x28/0x70
inet_release+0x8e/0xa7
__sock_release+0x95/0x121
sock_close+0x14/0x17
__fput+0x20f/0x36a
task_work_run+0xa3/0xcc
exit_to_user_mode_prepare+0x9c/0x14d
syscall_exit_to_user_mode+0x18/0x44
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Changes
=======
ver #2)
- Changed to suggestion from Hawkins Jiawei to have a ..._check() function
and make the original a special case of that.
Fixes: cf8c1e967224 ("net: refactor bpf_sk_reuseport_detach()")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Hawkins Jiawei <yin31149@gmail.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: netdev@vger.kernel.org
Link: https://lore.kernel.org/r/166064248071.3502205.10036394558814861778.stgit@warthog.procyon.org.uk # v1
---
include/net/sock.h | 18 ++++++++++++++----
kernel/bpf/reuseport_array.c | 3 ++-
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 05a1bbdf5805..6464da28e842 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -578,18 +578,22 @@ static inline bool sk_user_data_is_nocopy(const struct sock *sk)
#define __sk_user_data(sk) ((*((void __rcu **)&(sk)->sk_user_data)))
/**
- * __rcu_dereference_sk_user_data_with_flags - return the pointer
+ * __rcu_dereference_sk_user_data_with_flags_check - return the pointer
* only if argument flags all has been set in sk_user_data. Otherwise
* return NULL
*
* @sk: socket
* @flags: flag bits
+ * @condition: Condition under which non-RCU access may take place
+ *
+ * The caller must be holding the RCU read lock
*/
static inline void *
-__rcu_dereference_sk_user_data_with_flags(const struct sock *sk,
- uintptr_t flags)
+__rcu_dereference_sk_user_data_with_flags_check(const struct sock *sk,
+ uintptr_t flags, bool condition)
{
- uintptr_t sk_user_data = (uintptr_t)rcu_dereference(__sk_user_data(sk));
+ uintptr_t sk_user_data =
+ (uintptr_t)rcu_dereference_check(__sk_user_data(sk), condition);
WARN_ON_ONCE(flags & SK_USER_DATA_PTRMASK);
@@ -598,6 +602,12 @@ __rcu_dereference_sk_user_data_with_flags(const struct sock *sk,
return NULL;
}
+static inline void *
+__rcu_dereference_sk_user_data_with_flags(const struct sock *sk, uintptr_t flags)
+{
+ return __rcu_dereference_sk_user_data_with_flags_check(sk, flags, false);
+}
+
#define rcu_dereference_sk_user_data(sk) \
__rcu_dereference_sk_user_data_with_flags(sk, 0)
#define __rcu_assign_sk_user_data_with_flags(sk, ptr, flags) \
diff --git a/kernel/bpf/reuseport_array.c b/kernel/bpf/reuseport_array.c
index 85fa9dbfa8bf..856c360a591d 100644
--- a/kernel/bpf/reuseport_array.c
+++ b/kernel/bpf/reuseport_array.c
@@ -24,7 +24,8 @@ void bpf_sk_reuseport_detach(struct sock *sk)
struct sock __rcu **socks;
write_lock_bh(&sk->sk_callback_lock);
- socks = __rcu_dereference_sk_user_data_with_flags(sk, SK_USER_DATA_BPF);
+ socks = __rcu_dereference_sk_user_data_with_flags_check(
+ sk, SK_USER_DATA_BPF, lockdep_is_held(&sk->sk_callback_lock));
if (socks) {
WRITE_ONCE(sk->sk_user_data, NULL);
/*
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] net: Fix suspicious RCU usage in bpf_sk_reuseport_detach()
2022-08-16 13:26 [PATCH net v2] net: Fix suspicious RCU usage in bpf_sk_reuseport_detach() David Howells
@ 2022-08-17 5:19 ` kernel test robot
2022-08-17 8:13 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-08-17 5:19 UTC (permalink / raw)
To: David Howells, yin31149
Cc: kbuild-all, Jakub Kicinski, netdev, dhowells, linux-kernel
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/intel-lab-lkp/linux/commits/David-Howells/net-Fix-suspicious-RCU-usage-in-bpf_sk_reuseport_detach/20220816-212744
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git ae806c7805571a9813e41bf6763dd08d0706f4ed
config: nios2-buildonly-randconfig-r001-20220815 (https://download.01.org/0day-ci/archive/20220817/202208171301.3cD4S3Ui-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fe74fdc1e7fe8aa84006265deb7b55f40bcc8736
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review David-Howells/net-Fix-suspicious-RCU-usage-in-bpf_sk_reuseport_detach/20220816-212744
git checkout fe74fdc1e7fe8aa84006265deb7b55f40bcc8736
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=nios2 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
nios2-linux-ld: kernel/bpf/reuseport_array.o: in function `bpf_sk_reuseport_detach':
>> reuseport_array.c:(.text+0x368): undefined reference to `lockdep_is_held'
reuseport_array.c:(.text+0x368): relocation truncated to fit: R_NIOS2_CALL26 against `lockdep_is_held'
`adc3xxx_i2c_remove' referenced in section `.data' of sound/soc/codecs/tlv320adc3xxx.o: defined in discarded section `.exit.text' of sound/soc/codecs/tlv320adc3xxx.o
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] net: Fix suspicious RCU usage in bpf_sk_reuseport_detach()
2022-08-16 13:26 [PATCH net v2] net: Fix suspicious RCU usage in bpf_sk_reuseport_detach() David Howells
2022-08-17 5:19 ` kernel test robot
@ 2022-08-17 8:13 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-08-17 8:13 UTC (permalink / raw)
To: David Howells, yin31149
Cc: kbuild-all, Jakub Kicinski, netdev, dhowells, linux-kernel
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/intel-lab-lkp/linux/commits/David-Howells/net-Fix-suspicious-RCU-usage-in-bpf_sk_reuseport_detach/20220816-212744
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git ae806c7805571a9813e41bf6763dd08d0706f4ed
config: x86_64-rhel-8.3-kvm (https://download.01.org/0day-ci/archive/20220817/202208171521.JiKYmnhP-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/fe74fdc1e7fe8aa84006265deb7b55f40bcc8736
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review David-Howells/net-Fix-suspicious-RCU-usage-in-bpf_sk_reuseport_detach/20220816-212744
git checkout fe74fdc1e7fe8aa84006265deb7b55f40bcc8736
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
ld: kernel/bpf/reuseport_array.o: in function `bpf_sk_reuseport_detach':
>> kernel/bpf/reuseport_array.c:28: undefined reference to `lockdep_is_held'
vim +28 kernel/bpf/reuseport_array.c
20
21 /* The caller must hold the reuseport_lock */
22 void bpf_sk_reuseport_detach(struct sock *sk)
23 {
24 struct sock __rcu **socks;
25
26 write_lock_bh(&sk->sk_callback_lock);
27 socks = __rcu_dereference_sk_user_data_with_flags_check(
> 28 sk, SK_USER_DATA_BPF, lockdep_is_held(&sk->sk_callback_lock));
29 if (socks) {
30 WRITE_ONCE(sk->sk_user_data, NULL);
31 /*
32 * Do not move this NULL assignment outside of
33 * sk->sk_callback_lock because there is
34 * a race with reuseport_array_free()
35 * which does not hold the reuseport_lock.
36 */
37 RCU_INIT_POINTER(*socks, NULL);
38 }
39 write_unlock_bh(&sk->sk_callback_lock);
40 }
41
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-17 8:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-16 13:26 [PATCH net v2] net: Fix suspicious RCU usage in bpf_sk_reuseport_detach() David Howells
2022-08-17 5:19 ` kernel test robot
2022-08-17 8:13 ` kernel test robot
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).