diff options
author | Xin Long <lucien.xin@gmail.com> | 2019-10-08 07:09:23 -0400 |
---|---|---|
committer | Jakub Kicinski <jakub.kicinski@netronome.com> | 2019-10-09 19:27:04 -0400 |
commit | 819be8108fded0b9e710bbbf81193e52f7bab2f7 (patch) | |
tree | 435781d818895e15928e95b7005eede3ded2e1d0 /net | |
parent | a7137534b597b7c303203e6bc3ed87e87a273bb8 (diff) |
sctp: add chunks to sk_backlog when the newsk sk_socket is not set
This patch is to fix a NULL-ptr deref in selinux_socket_connect_helper:
[...] kasan: GPF could be caused by NULL-ptr deref or user memory access
[...] RIP: 0010:selinux_socket_connect_helper+0x94/0x460
[...] Call Trace:
[...] selinux_sctp_bind_connect+0x16a/0x1d0
[...] security_sctp_bind_connect+0x58/0x90
[...] sctp_process_asconf+0xa52/0xfd0 [sctp]
[...] sctp_sf_do_asconf+0x785/0x980 [sctp]
[...] sctp_do_sm+0x175/0x5a0 [sctp]
[...] sctp_assoc_bh_rcv+0x285/0x5b0 [sctp]
[...] sctp_backlog_rcv+0x482/0x910 [sctp]
[...] __release_sock+0x11e/0x310
[...] release_sock+0x4f/0x180
[...] sctp_accept+0x3f9/0x5a0 [sctp]
[...] inet_accept+0xe7/0x720
It was caused by that the 'newsk' sk_socket was not set before going to
security sctp hook when processing asconf chunk with SCTP_PARAM_ADD_IP
or SCTP_PARAM_SET_PRIMARY:
inet_accept()->
sctp_accept():
lock_sock():
lock listening 'sk'
do_softirq():
sctp_rcv(): <-- [1]
asconf chunk arrives and
enqueued in 'sk' backlog
sctp_sock_migrate():
set asoc's sk to 'newsk'
release_sock():
sctp_backlog_rcv():
lock 'newsk'
sctp_process_asconf() <-- [2]
unlock 'newsk'
sock_graft():
set sk_socket <-- [3]
As it shows, at [1] the asconf chunk would be put into the listening 'sk'
backlog, as accept() was holding its sock lock. Then at [2] asconf would
get processed with 'newsk' as asoc's sk had been set to 'newsk'. However,
'newsk' sk_socket is not set until [3], while selinux_sctp_bind_connect()
would deref it, then kernel crashed.
Here to fix it by adding the chunk to sk_backlog until newsk sk_socket is
set when .accept() is done.
Note that sk->sk_socket can be NULL when the sock is closed, so SOCK_DEAD
flag is also needed to check in sctp_newsk_ready().
Thanks to Ondrej for reviewing the code.
Fixes: d452930fd3b9 ("selinux: Add SCTP support")
Reported-by: Ying Xu <yinxu@redhat.com>
Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Diffstat (limited to 'net')
-rw-r--r-- | net/sctp/input.c | 12 |
1 files changed, 9 insertions, 3 deletions
diff --git a/net/sctp/input.c b/net/sctp/input.c index 5a070fb5b278..f2771375bfc0 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c | |||
@@ -243,7 +243,7 @@ int sctp_rcv(struct sk_buff *skb) | |||
243 | bh_lock_sock(sk); | 243 | bh_lock_sock(sk); |
244 | } | 244 | } |
245 | 245 | ||
246 | if (sock_owned_by_user(sk)) { | 246 | if (sock_owned_by_user(sk) || !sctp_newsk_ready(sk)) { |
247 | if (sctp_add_backlog(sk, skb)) { | 247 | if (sctp_add_backlog(sk, skb)) { |
248 | bh_unlock_sock(sk); | 248 | bh_unlock_sock(sk); |
249 | sctp_chunk_free(chunk); | 249 | sctp_chunk_free(chunk); |
@@ -321,7 +321,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb) | |||
321 | local_bh_disable(); | 321 | local_bh_disable(); |
322 | bh_lock_sock(sk); | 322 | bh_lock_sock(sk); |
323 | 323 | ||
324 | if (sock_owned_by_user(sk)) { | 324 | if (sock_owned_by_user(sk) || !sctp_newsk_ready(sk)) { |
325 | if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) | 325 | if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) |
326 | sctp_chunk_free(chunk); | 326 | sctp_chunk_free(chunk); |
327 | else | 327 | else |
@@ -336,7 +336,13 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb) | |||
336 | if (backloged) | 336 | if (backloged) |
337 | return 0; | 337 | return 0; |
338 | } else { | 338 | } else { |
339 | sctp_inq_push(inqueue, chunk); | 339 | if (!sctp_newsk_ready(sk)) { |
340 | if (!sk_add_backlog(sk, skb, sk->sk_rcvbuf)) | ||
341 | return 0; | ||
342 | sctp_chunk_free(chunk); | ||
343 | } else { | ||
344 | sctp_inq_push(inqueue, chunk); | ||
345 | } | ||
340 | } | 346 | } |
341 | 347 | ||
342 | done: | 348 | done: |