Message ID | 20180312210412.1875-1-tom@quantonium.net |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
Series | [net] kcm: lock lower socket in kcm_attach | expand |
On Mon, Mar 12, 2018 at 02:04:12PM -0700, Tom Herbert wrote: > Need to lock lower socket in order to provide mutual exclusion > with kcm_unattach. > > Fixes: ab7ac4eb9832e32a09f4e804 ("kcm: Kernel Connection Multiplexor module") > Signed-off-by: Tom Herbert <tom@quantonium.net> > --- Is this fixing the syzbot-reported bug "KASAN: use-after-free Read in get_work_pool"? If so, please add: Reported-by: syzbot+ea75c0ffcd353d32515f064aaebefc5279e6161e@syzkaller.appspotmail.com
On Mon, Mar 12, 2018 at 2:09 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > On Mon, Mar 12, 2018 at 02:04:12PM -0700, Tom Herbert wrote: >> Need to lock lower socket in order to provide mutual exclusion >> with kcm_unattach. >> >> Fixes: ab7ac4eb9832e32a09f4e804 ("kcm: Kernel Connection Multiplexor module") >> Signed-off-by: Tom Herbert <tom@quantonium.net> >> --- > > Is this fixing the syzbot-reported bug "KASAN: use-after-free Read in > get_work_pool"? If so, please add: > > Reported-by: syzbot+ea75c0ffcd353d32515f064aaebefc5279e6161e@syzkaller.appspotmail.com Yeah, I was looking for a "reported by". I didn't see it in the email from syzbot. Where is this found? Thanks, Tom
On Mon, Mar 12, 2018 at 02:25:41PM -0700, Tom Herbert wrote: > On Mon, Mar 12, 2018 at 2:09 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > > On Mon, Mar 12, 2018 at 02:04:12PM -0700, Tom Herbert wrote: > >> Need to lock lower socket in order to provide mutual exclusion > >> with kcm_unattach. > >> > >> Fixes: ab7ac4eb9832e32a09f4e804 ("kcm: Kernel Connection Multiplexor module") > >> Signed-off-by: Tom Herbert <tom@quantonium.net> > >> --- > > > > Is this fixing the syzbot-reported bug "KASAN: use-after-free Read in > > get_work_pool"? If so, please add: > > > > Reported-by: syzbot+ea75c0ffcd353d32515f064aaebefc5279e6161e@syzkaller.appspotmail.com > > Yeah, I was looking for a "reported by". I didn't see it in the email > from syzbot. Where is this found? > > Thanks, > Tom This was an old bug report that was sent out before syzbot was updated to suggest a Reported-by line. But you can still use Reported-by for these old bugs. I took the bug ID from the From: header of the email. Eric
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c index f297d53a11aa..34355fd19f27 100644 --- a/net/kcm/kcmsock.c +++ b/net/kcm/kcmsock.c @@ -1381,24 +1381,32 @@ static int kcm_attach(struct socket *sock, struct socket *csock, .parse_msg = kcm_parse_func_strparser, .read_sock_done = kcm_read_sock_done, }; - int err; + int err = 0; csk = csock->sk; if (!csk) return -EINVAL; + lock_sock(csk); + /* Only allow TCP sockets to be attached for now */ if ((csk->sk_family != AF_INET && csk->sk_family != AF_INET6) || - csk->sk_protocol != IPPROTO_TCP) - return -EOPNOTSUPP; + csk->sk_protocol != IPPROTO_TCP) { + err = -EOPNOTSUPP; + goto out; + } /* Don't allow listeners or closed sockets */ - if (csk->sk_state == TCP_LISTEN || csk->sk_state == TCP_CLOSE) - return -EOPNOTSUPP; + if (csk->sk_state == TCP_LISTEN || csk->sk_state == TCP_CLOSE) { + err = -EOPNOTSUPP; + goto out; + } psock = kmem_cache_zalloc(kcm_psockp, GFP_KERNEL); - if (!psock) - return -ENOMEM; + if (!psock) { + err = -ENOMEM; + goto out; + } psock->mux = mux; psock->sk = csk; @@ -1407,7 +1415,7 @@ static int kcm_attach(struct socket *sock, struct socket *csock, err = strp_init(&psock->strp, csk, &cb); if (err) { kmem_cache_free(kcm_psockp, psock); - return err; + goto out; } write_lock_bh(&csk->sk_callback_lock); @@ -1419,7 +1427,8 @@ static int kcm_attach(struct socket *sock, struct socket *csock, write_unlock_bh(&csk->sk_callback_lock); strp_done(&psock->strp); kmem_cache_free(kcm_psockp, psock); - return -EALREADY; + err = -EALREADY; + goto out; } psock->save_data_ready = csk->sk_data_ready; @@ -1455,7 +1464,10 @@ static int kcm_attach(struct socket *sock, struct socket *csock, /* Schedule RX work in case there are already bytes queued */ strp_check_rcv(&psock->strp); - return 0; +out: + release_sock(csk); + + return err; } static int kcm_attach_ioctl(struct socket *sock, struct kcm_attach *info) @@ -1507,6 +1519,7 @@ static void kcm_unattach(struct kcm_psock *psock) if (WARN_ON(psock->rx_kcm)) { write_unlock_bh(&csk->sk_callback_lock); + release_sock(csk); return; }
Need to lock lower socket in order to provide mutual exclusion with kcm_unattach. Fixes: ab7ac4eb9832e32a09f4e804 ("kcm: Kernel Connection Multiplexor module") Signed-off-by: Tom Herbert <tom@quantonium.net> --- net/kcm/kcmsock.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)