diff options
| author | Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> | 2018-10-16 15:18:17 -0300 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2018-11-10 07:42:58 -0800 |
| commit | 1b0bb7e515b2292da2be1286b994ad1cf1096af3 (patch) | |
| tree | 578bbcaa64a4ea223898b56527156286579cc90c /net/sctp | |
| parent | 01a2ff110120315b768f63a10cffb7ae48d68902 (diff) | |
| download | linux-stable-1b0bb7e515b2292da2be1286b994ad1cf1096af3.tar.gz linux-stable-1b0bb7e515b2292da2be1286b994ad1cf1096af3.tar.bz2 linux-stable-1b0bb7e515b2292da2be1286b994ad1cf1096af3.zip | |
sctp: fix race on sctp_id2asoc
[ Upstream commit b336decab22158937975293aea79396525f92bb3 ]
syzbot reported an use-after-free involving sctp_id2asoc. Dmitry Vyukov
helped to root cause it and it is because of reading the asoc after it
was freed:
CPU 1 CPU 2
(working on socket 1) (working on socket 2)
sctp_association_destroy
sctp_id2asoc
spin lock
grab the asoc from idr
spin unlock
spin lock
remove asoc from idr
spin unlock
free(asoc)
if asoc->base.sk != sk ... [*]
This can only be hit if trying to fetch asocs from different sockets. As
we have a single IDR for all asocs, in all SCTP sockets, their id is
unique on the system. An application can try to send stuff on an id
that matches on another socket, and the if in [*] will protect from such
usage. But it didn't consider that as that asoc may belong to another
socket, it may be freed in parallel (read: under another socket lock).
We fix it by moving the checks in [*] into the protected region. This
fixes it because the asoc cannot be freed while the lock is held.
Reported-by: syzbot+c7dd55d7aec49d48e49a@syzkaller.appspotmail.com
Acked-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'net/sctp')
| -rw-r--r-- | net/sctp/socket.c | 5 |
1 files changed, 2 insertions, 3 deletions
diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 16aa8673f918..9827ba4b9f74 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -248,11 +248,10 @@ struct sctp_association *sctp_id2assoc(struct sock *sk, sctp_assoc_t id) spin_lock_bh(&sctp_assocs_id_lock); asoc = (struct sctp_association *)idr_find(&sctp_assocs_id, (int)id); + if (asoc && (asoc->base.sk != sk || asoc->base.dead)) + asoc = NULL; spin_unlock_bh(&sctp_assocs_id_lock); - if (!asoc || (asoc->base.sk != sk) || asoc->base.dead) - return NULL; - return asoc; } |