Skip to content

Commit 50e741b

Browse files
Florian Westphaldavem330
authored andcommitted
mptcp: fix panic on user pointer access
Its not possible to call the kernel_(s|g)etsockopt functions here, the address points to user memory: General protection fault in user access. Non-canonical address? WARNING: CPU: 1 PID: 5352 at arch/x86/mm/extable.c:77 ex_handler_uaccess+0xba/0xe0 arch/x86/mm/extable.c:77 Kernel panic - not syncing: panic_on_warn set ... [..] Call Trace: fixup_exception+0x9d/0xcd arch/x86/mm/extable.c:178 general_protection+0x2d/0x40 arch/x86/entry/entry_64.S:1202 do_ip_getsockopt+0x1f6/0x1860 net/ipv4/ip_sockglue.c:1323 ip_getsockopt+0x87/0x1c0 net/ipv4/ip_sockglue.c:1561 tcp_getsockopt net/ipv4/tcp.c:3691 [inline] tcp_getsockopt+0x8c/0xd0 net/ipv4/tcp.c:3685 kernel_getsockopt+0x121/0x1f0 net/socket.c:3736 mptcp_getsockopt+0x69/0x90 net/mptcp/protocol.c:830 __sys_getsockopt+0x13a/0x220 net/socket.c:2175 We can call tcp_get/setsockopt functions instead. Doing so fixes crashing, but still leaves rtnl related lockdep splat: WARNING: possible circular locking dependency detected 5.5.0-rc6 #2 Not tainted ------------------------------------------------------ syz-executor.0/16334 is trying to acquire lock: ffffffff84f7a080 (rtnl_mutex){+.+.}, at: do_ip_setsockopt.isra.0+0x277/0x3820 net/ipv4/ip_sockglue.c:644 but task is already holding lock: ffff888116503b90 (sk_lock-AF_INET){+.+.}, at: lock_sock include/net/sock.h:1516 [inline] ffff888116503b90 (sk_lock-AF_INET){+.+.}, at: mptcp_setsockopt+0x28/0x90 net/mptcp/protocol.c:1284 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (sk_lock-AF_INET){+.+.}: lock_sock_nested+0xca/0x120 net/core/sock.c:2944 lock_sock include/net/sock.h:1516 [inline] do_ip_setsockopt.isra.0+0x281/0x3820 net/ipv4/ip_sockglue.c:645 ip_setsockopt+0x44/0xf0 net/ipv4/ip_sockglue.c:1248 udp_setsockopt+0x5d/0xa0 net/ipv4/udp.c:2639 __sys_setsockopt+0x152/0x240 net/socket.c:2130 __do_sys_setsockopt net/socket.c:2146 [inline] __se_sys_setsockopt net/socket.c:2143 [inline] __x64_sys_setsockopt+0xba/0x150 net/socket.c:2143 do_syscall_64+0xbd/0x5b0 arch/x86/entry/common.c:294 entry_SYSCALL_64_after_hwframe+0x49/0xbe -> #0 (rtnl_mutex){+.+.}: check_prev_add kernel/locking/lockdep.c:2475 [inline] check_prevs_add kernel/locking/lockdep.c:2580 [inline] validate_chain kernel/locking/lockdep.c:2970 [inline] __lock_acquire+0x1fb2/0x4680 kernel/locking/lockdep.c:3954 lock_acquire+0x127/0x330 kernel/locking/lockdep.c:4484 __mutex_lock_common kernel/locking/mutex.c:956 [inline] __mutex_lock+0x158/0x1340 kernel/locking/mutex.c:1103 do_ip_setsockopt.isra.0+0x277/0x3820 net/ipv4/ip_sockglue.c:644 ip_setsockopt+0x44/0xf0 net/ipv4/ip_sockglue.c:1248 tcp_setsockopt net/ipv4/tcp.c:3159 [inline] tcp_setsockopt+0x8c/0xd0 net/ipv4/tcp.c:3153 kernel_setsockopt+0x121/0x1f0 net/socket.c:3767 mptcp_setsockopt+0x69/0x90 net/mptcp/protocol.c:1288 __sys_setsockopt+0x152/0x240 net/socket.c:2130 __do_sys_setsockopt net/socket.c:2146 [inline] __se_sys_setsockopt net/socket.c:2143 [inline] __x64_sys_setsockopt+0xba/0x150 net/socket.c:2143 do_syscall_64+0xbd/0x5b0 arch/x86/entry/common.c:294 entry_SYSCALL_64_after_hwframe+0x49/0xbe other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(sk_lock-AF_INET); lock(rtnl_mutex); lock(sk_lock-AF_INET); lock(rtnl_mutex); The lockdep complaint is because we hold mptcp socket lock when calling the sk_prot get/setsockopt handler, and those might need to acquire the rtnl mutex. Normally, order is: rtnl_lock(sk) -> lock_sock Whereas for mptcp the order is lock_sock(mptcp_sk) rtnl_lock -> lock_sock(subflow_sk) We can avoid this by releasing the mptcp socket lock early, but, as Paolo points out, we need to get/put the subflow socket refcount before doing so to avoid race with concurrent close(). Fixes: 717e79c ("mptcp: Add setsockopt()/getsockopt() socket operations") Reported-by: Christoph Paasch <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent c9fd9c5 commit 50e741b

File tree

1 file changed

+22
-18
lines changed

1 file changed

+22
-18
lines changed

net/mptcp/protocol.c

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -781,15 +781,12 @@ static void mptcp_destroy(struct sock *sk)
781781
}
782782

783783
static int mptcp_setsockopt(struct sock *sk, int level, int optname,
784-
char __user *uoptval, unsigned int optlen)
784+
char __user *optval, unsigned int optlen)
785785
{
786786
struct mptcp_sock *msk = mptcp_sk(sk);
787-
char __kernel *optval;
788787
int ret = -EOPNOTSUPP;
789788
struct socket *ssock;
790-
791-
/* will be treated as __user in tcp_setsockopt */
792-
optval = (char __kernel __force *)uoptval;
789+
struct sock *ssk;
793790

794791
pr_debug("msk=%p", msk);
795792

@@ -798,27 +795,28 @@ static int mptcp_setsockopt(struct sock *sk, int level, int optname,
798795
*/
799796
lock_sock(sk);
800797
ssock = __mptcp_socket_create(msk, MPTCP_SAME_STATE);
801-
if (!IS_ERR(ssock)) {
802-
pr_debug("subflow=%p", ssock->sk);
803-
ret = kernel_setsockopt(ssock, level, optname, optval, optlen);
798+
if (IS_ERR(ssock)) {
799+
release_sock(sk);
800+
return ret;
804801
}
802+
803+
ssk = ssock->sk;
804+
sock_hold(ssk);
805805
release_sock(sk);
806806

807+
ret = tcp_setsockopt(ssk, level, optname, optval, optlen);
808+
sock_put(ssk);
809+
807810
return ret;
808811
}
809812

810813
static int mptcp_getsockopt(struct sock *sk, int level, int optname,
811-
char __user *uoptval, int __user *uoption)
814+
char __user *optval, int __user *option)
812815
{
813816
struct mptcp_sock *msk = mptcp_sk(sk);
814-
char __kernel *optval;
815817
int ret = -EOPNOTSUPP;
816-
int __kernel *option;
817818
struct socket *ssock;
818-
819-
/* will be treated as __user in tcp_getsockopt */
820-
optval = (char __kernel __force *)uoptval;
821-
option = (int __kernel __force *)uoption;
819+
struct sock *ssk;
822820

823821
pr_debug("msk=%p", msk);
824822

@@ -827,12 +825,18 @@ static int mptcp_getsockopt(struct sock *sk, int level, int optname,
827825
*/
828826
lock_sock(sk);
829827
ssock = __mptcp_socket_create(msk, MPTCP_SAME_STATE);
830-
if (!IS_ERR(ssock)) {
831-
pr_debug("subflow=%p", ssock->sk);
832-
ret = kernel_getsockopt(ssock, level, optname, optval, option);
828+
if (IS_ERR(ssock)) {
829+
release_sock(sk);
830+
return ret;
833831
}
832+
833+
ssk = ssock->sk;
834+
sock_hold(ssk);
834835
release_sock(sk);
835836

837+
ret = tcp_getsockopt(ssk, level, optname, optval, option);
838+
sock_put(ssk);
839+
836840
return ret;
837841
}
838842

0 commit comments

Comments
 (0)