| Message ID | 20170321195415.8021-1-cascardo@canonical.com |
|---|---|
| State | New |
| Headers | show |
I don't see a buglink nor do I find a launchpad bug for this CVE. Both are required for an SRU. On Tue, Mar 21, 2017 at 04:54:15PM -0300, Thadeu Lima de Souza Cascardo wrote: > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > commit 2dcab5984841 ("sctp: avoid BUG_ON on sctp_wait_for_sndbuf") > attempted to avoid a BUG_ON call when the association being used for a > sendmsg() is blocked waiting for more sndbuf and another thread did a > peeloff operation on such asoc, moving it to another socket. > > As Ben Hutchings noticed, then in such case it would return without > locking back the socket and would cause two unlocks in a row. > > Further analysis also revealed that it could allow a double free if the > application managed to peeloff the asoc that is created during the > sendmsg call, because then sctp_sendmsg() would try to free the asoc > that was created only for that call. > > This patch takes another approach. It will deny the peeloff operation > if there is a thread sleeping on the asoc, so this situation doesn't > exist anymore. This avoids the issues described above and also honors > the syscalls that are already being handled (it can be multiple sendmsg > calls). > > Joint work with Xin Long. > > Fixes: 2dcab5984841 ("sctp: avoid BUG_ON on sctp_wait_for_sndbuf") > Cc: Alexander Popov <alex.popov@linux.com> > Cc: Ben Hutchings <ben@decadent.org.uk> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: Xin Long <lucien.xin@gmail.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > (cherry picked from commit dfcb9f4f99f1e9a49e43398a7bfbf56927544af1) > CVE-2017-6353 > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- > net/sctp/socket.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 138f2d6..5758818 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -4422,6 +4422,12 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp) > if (!asoc) > return -EINVAL; > > + /* If there is a thread waiting on more sndbuf space for > + * sending on this asoc, it cannot be peeled. > + */ > + if (waitqueue_active(&asoc->wait)) > + return -EBUSY; > + > /* An association cannot be branched off from an already peeled-off > * socket, nor is this supported for tcp style sockets. > */ > @@ -6960,8 +6966,6 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p, > */ > release_sock(sk); > current_timeo = schedule_timeout(current_timeo); > - if (sk != asoc->base.sk) > - goto do_error; > lock_sock(sk); > > *timeo_p = current_timeo; > -- > 2.9.3 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Wed, Mar 29, 2017 at 03:50:03PM -0700, Brad Figg wrote: > > I don't see a buglink nor do I find a launchpad bug for this CVE. Both are > required for an SRU. > I assumed that as the patch has the CVE line, a bug was not required. BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1677579 Cascardo. > On Tue, Mar 21, 2017 at 04:54:15PM -0300, Thadeu Lima de Souza Cascardo wrote: > > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > > > commit 2dcab5984841 ("sctp: avoid BUG_ON on sctp_wait_for_sndbuf") > > attempted to avoid a BUG_ON call when the association being used for a > > sendmsg() is blocked waiting for more sndbuf and another thread did a > > peeloff operation on such asoc, moving it to another socket. > > > > As Ben Hutchings noticed, then in such case it would return without > > locking back the socket and would cause two unlocks in a row. > > > > Further analysis also revealed that it could allow a double free if the > > application managed to peeloff the asoc that is created during the > > sendmsg call, because then sctp_sendmsg() would try to free the asoc > > that was created only for that call. > > > > This patch takes another approach. It will deny the peeloff operation > > if there is a thread sleeping on the asoc, so this situation doesn't > > exist anymore. This avoids the issues described above and also honors > > the syscalls that are already being handled (it can be multiple sendmsg > > calls). > > > > Joint work with Xin Long. > > > > Fixes: 2dcab5984841 ("sctp: avoid BUG_ON on sctp_wait_for_sndbuf") > > Cc: Alexander Popov <alex.popov@linux.com> > > Cc: Ben Hutchings <ben@decadent.org.uk> > > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > Signed-off-by: David S. Miller <davem@davemloft.net> > > (cherry picked from commit dfcb9f4f99f1e9a49e43398a7bfbf56927544af1) > > CVE-2017-6353 > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > > --- > > net/sctp/socket.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > > index 138f2d6..5758818 100644 > > --- a/net/sctp/socket.c > > +++ b/net/sctp/socket.c > > @@ -4422,6 +4422,12 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp) > > if (!asoc) > > return -EINVAL; > > > > + /* If there is a thread waiting on more sndbuf space for > > + * sending on this asoc, it cannot be peeled. > > + */ > > + if (waitqueue_active(&asoc->wait)) > > + return -EBUSY; > > + > > /* An association cannot be branched off from an already peeled-off > > * socket, nor is this supported for tcp style sockets. > > */ > > @@ -6960,8 +6966,6 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p, > > */ > > release_sock(sk); > > current_timeo = schedule_timeout(current_timeo); > > - if (sk != asoc->base.sk) > > - goto do_error; > > lock_sock(sk); > > > > *timeo_p = current_timeo; > > -- > > 2.9.3 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team > > -- > Brad Figg brad.figg@canonical.com http://www.canonical.com
Need to add the BugLink
Applied to xenial master-next branch. Thanks. Cascardo.
diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 138f2d6..5758818 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -4422,6 +4422,12 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp) if (!asoc) return -EINVAL; + /* If there is a thread waiting on more sndbuf space for + * sending on this asoc, it cannot be peeled. + */ + if (waitqueue_active(&asoc->wait)) + return -EBUSY; + /* An association cannot be branched off from an already peeled-off * socket, nor is this supported for tcp style sockets. */ @@ -6960,8 +6966,6 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p, */ release_sock(sk); current_timeo = schedule_timeout(current_timeo); - if (sk != asoc->base.sk) - goto do_error; lock_sock(sk); *timeo_p = current_timeo;