Skip to content

Commit d7ef969

Browse files
Paolo Abenigregkh
Paolo Abeni
authored andcommitted
netfilter: drop outermost socket lock in getsockopt()
commit 01ea306 upstream. The Syzbot reported a possible deadlock in the netfilter area caused by rtnl lock, xt lock and socket lock being acquired with a different order on different code paths, leading to the following backtrace: Reviewed-by: Xin Long <[email protected]> ====================================================== WARNING: possible circular locking dependency detected 4.15.0+ #301 Not tainted ------------------------------------------------------ syzkaller233489/4179 is trying to acquire lock: (rtnl_mutex){+.+.}, at: [<0000000048e996fd>] rtnl_lock+0x17/0x20 net/core/rtnetlink.c:74 but task is already holding lock: (&xt[i].mutex){+.+.}, at: [<00000000328553a2>] xt_find_table_lock+0x3e/0x3e0 net/netfilter/x_tables.c:1041 which lock already depends on the new lock. === Since commit 3f34cfae1230 ("netfilter: on sockopt() acquire sock lock only in the required scope"), we already acquire the socket lock in the innermost scope, where needed. In such commit I forgot to remove the outer-most socket lock from the getsockopt() path, this commit addresses the issues dropping it now. v1 -> v2: fix bad subj, added relavant 'fixes' tag Fixes: 22265a5 ("netfilter: xt_TEE: resolve oif using netdevice notifiers") Fixes: 202f59a ("netfilter: ipt_CLUSTERIP: do not hold dev") Fixes: 3f34cfae1230 ("netfilter: on sockopt() acquire sock lock only in the required scope") Reported-by: [email protected] Suggested-by: Florian Westphal <[email protected]> Signed-off-by: Paolo Abeni <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> Tested-by: Krzysztof Piotr Oledzki <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 1a7aef6 commit d7ef969

File tree

2 files changed

+3
-14
lines changed

2 files changed

+3
-14
lines changed

net/ipv4/ip_sockglue.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1563,10 +1563,7 @@ int ip_getsockopt(struct sock *sk, int level,
15631563
if (get_user(len, optlen))
15641564
return -EFAULT;
15651565

1566-
lock_sock(sk);
1567-
err = nf_getsockopt(sk, PF_INET, optname, optval,
1568-
&len);
1569-
release_sock(sk);
1566+
err = nf_getsockopt(sk, PF_INET, optname, optval, &len);
15701567
if (err >= 0)
15711568
err = put_user(len, optlen);
15721569
return err;
@@ -1598,9 +1595,7 @@ int compat_ip_getsockopt(struct sock *sk, int level, int optname,
15981595
if (get_user(len, optlen))
15991596
return -EFAULT;
16001597

1601-
lock_sock(sk);
16021598
err = compat_nf_getsockopt(sk, PF_INET, optname, optval, &len);
1603-
release_sock(sk);
16041599
if (err >= 0)
16051600
err = put_user(len, optlen);
16061601
return err;

net/ipv6/ipv6_sockglue.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,10 +1367,7 @@ int ipv6_getsockopt(struct sock *sk, int level, int optname,
13671367
if (get_user(len, optlen))
13681368
return -EFAULT;
13691369

1370-
lock_sock(sk);
1371-
err = nf_getsockopt(sk, PF_INET6, optname, optval,
1372-
&len);
1373-
release_sock(sk);
1370+
err = nf_getsockopt(sk, PF_INET6, optname, optval, &len);
13741371
if (err >= 0)
13751372
err = put_user(len, optlen);
13761373
}
@@ -1409,10 +1406,7 @@ int compat_ipv6_getsockopt(struct sock *sk, int level, int optname,
14091406
if (get_user(len, optlen))
14101407
return -EFAULT;
14111408

1412-
lock_sock(sk);
1413-
err = compat_nf_getsockopt(sk, PF_INET6,
1414-
optname, optval, &len);
1415-
release_sock(sk);
1409+
err = compat_nf_getsockopt(sk, PF_INET6, optname, optval, &len);
14161410
if (err >= 0)
14171411
err = put_user(len, optlen);
14181412
}

0 commit comments

Comments
 (0)