Skip to content

Commit e198987

Browse files
TaeheeYoodavem330
authored andcommitted
gtp: fix suspicious RCU usage
gtp_encap_enable_socket() and gtp_encap_destroy() are not protected by rcu_read_lock(). and it's not safe to write sk->sk_user_data. This patch make these functions to use lock_sock() instead of rcu_dereference_sk_user_data(). Test commands: gtp-link add gtp1 Splat looks like: [ 83.238315] ============================= [ 83.239127] WARNING: suspicious RCU usage [ 83.239702] 5.2.0-rc6+ #49 Not tainted [ 83.240268] ----------------------------- [ 83.241205] drivers/net/gtp.c:799 suspicious rcu_dereference_check() usage! [ 83.243828] [ 83.243828] other info that might help us debug this: [ 83.243828] [ 83.246325] [ 83.246325] rcu_scheduler_active = 2, debug_locks = 1 [ 83.247314] 1 lock held by gtp-link/1008: [ 83.248523] #0: 0000000017772c7f (rtnl_mutex){+.+.}, at: __rtnl_newlink+0x5f5/0x11b0 [ 83.251503] [ 83.251503] stack backtrace: [ 83.252173] CPU: 0 PID: 1008 Comm: gtp-link Not tainted 5.2.0-rc6+ #49 [ 83.253271] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 83.254562] Call Trace: [ 83.254995] dump_stack+0x7c/0xbb [ 83.255567] gtp_encap_enable_socket+0x2df/0x360 [gtp] [ 83.256415] ? gtp_find_dev+0x1a0/0x1a0 [gtp] [ 83.257161] ? memset+0x1f/0x40 [ 83.257843] gtp_newlink+0x90/0xa21 [gtp] [ 83.258497] ? __netlink_ns_capable+0xc3/0xf0 [ 83.259260] __rtnl_newlink+0xb9f/0x11b0 [ 83.260022] ? rtnl_link_unregister+0x230/0x230 [ ... ] Fixes: 1e3a3ab ("gtp: make GTP sockets in gtp_newlink optional") Signed-off-by: Taehee Yoo <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent ccd1479 commit e198987

File tree

1 file changed

+6
-2
lines changed

1 file changed

+6
-2
lines changed

drivers/net/gtp.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,12 +289,14 @@ static void gtp_encap_destroy(struct sock *sk)
289289
{
290290
struct gtp_dev *gtp;
291291

292-
gtp = rcu_dereference_sk_user_data(sk);
292+
lock_sock(sk);
293+
gtp = sk->sk_user_data;
293294
if (gtp) {
294295
udp_sk(sk)->encap_type = 0;
295296
rcu_assign_sk_user_data(sk, NULL);
296297
sock_put(sk);
297298
}
299+
release_sock(sk);
298300
}
299301

300302
static void gtp_encap_disable_sock(struct sock *sk)
@@ -796,7 +798,8 @@ static struct sock *gtp_encap_enable_socket(int fd, int type,
796798
goto out_sock;
797799
}
798800

799-
if (rcu_dereference_sk_user_data(sock->sk)) {
801+
lock_sock(sock->sk);
802+
if (sock->sk->sk_user_data) {
800803
sk = ERR_PTR(-EBUSY);
801804
goto out_sock;
802805
}
@@ -812,6 +815,7 @@ static struct sock *gtp_encap_enable_socket(int fd, int type,
812815
setup_udp_tunnel_sock(sock_net(sock->sk), sock, &tuncfg);
813816

814817
out_sock:
818+
release_sock(sock->sk);
815819
sockfd_put(sock);
816820
return sk;
817821
}

0 commit comments

Comments
 (0)