Message ID | 20190613103559.2603-1-nhorman@tuxdriver.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [v5,net] sctp: Free cookie before we memdup a new one | expand |
On Thu, Jun 13, 2019 at 06:35:59AM -0400, Neil Horman wrote: > Based on comments from Xin, even after fixes for our recent syzbot > report of cookie memory leaks, its possible to get a resend of an INIT > chunk which would lead to us leaking cookie memory. > > To ensure that we don't leak cookie memory, free any previously > allocated cookie first. > > Change notes > v1->v2 > update subsystem tag in subject (davem) > repeat kfree check for peer_random and peer_hmacs (xin) > > v2->v3 > net->sctp > also free peer_chunks > > v3->v4 > fix subject tags > > v4->v5 > remove cut line > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > Reported-by: syzbot+f7e9153b037eac9b1df8@syzkaller.appspotmail.com > CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > CC: Xin Long <lucien.xin@gmail.com> > CC: "David S. Miller" <davem@davemloft.net> > CC: netdev@vger.kernel.org Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > --- > net/sctp/sm_make_chunk.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c > index f17908f5c4f3..9b0e5b0d701a 100644 > --- a/net/sctp/sm_make_chunk.c > +++ b/net/sctp/sm_make_chunk.c > @@ -2583,6 +2583,8 @@ static int sctp_process_param(struct sctp_association *asoc, > case SCTP_PARAM_STATE_COOKIE: > asoc->peer.cookie_len = > ntohs(param.p->length) - sizeof(struct sctp_paramhdr); > + if (asoc->peer.cookie) > + kfree(asoc->peer.cookie); > asoc->peer.cookie = kmemdup(param.cookie->body, asoc->peer.cookie_len, gfp); > if (!asoc->peer.cookie) > retval = 0; > @@ -2647,6 +2649,8 @@ static int sctp_process_param(struct sctp_association *asoc, > goto fall_through; > > /* Save peer's random parameter */ > + if (asoc->peer.peer_random) > + kfree(asoc->peer.peer_random); > asoc->peer.peer_random = kmemdup(param.p, > ntohs(param.p->length), gfp); > if (!asoc->peer.peer_random) { > @@ -2660,6 +2664,8 @@ static int sctp_process_param(struct sctp_association *asoc, > goto fall_through; > > /* Save peer's HMAC list */ > + if (asoc->peer.peer_hmacs) > + kfree(asoc->peer.peer_hmacs); > asoc->peer.peer_hmacs = kmemdup(param.p, > ntohs(param.p->length), gfp); > if (!asoc->peer.peer_hmacs) { > @@ -2675,6 +2681,8 @@ static int sctp_process_param(struct sctp_association *asoc, > if (!ep->auth_enable) > goto fall_through; > > + if (asoc->peer.peer_chunks) > + kfree(asoc->peer.peer_chunks); > asoc->peer.peer_chunks = kmemdup(param.p, > ntohs(param.p->length), gfp); > if (!asoc->peer.peer_chunks) > -- > 2.20.1 >
From: Neil Horman <nhorman@tuxdriver.com> Date: Thu, 13 Jun 2019 06:35:59 -0400 > Based on comments from Xin, even after fixes for our recent syzbot > report of cookie memory leaks, its possible to get a resend of an INIT > chunk which would lead to us leaking cookie memory. > > To ensure that we don't leak cookie memory, free any previously > allocated cookie first. > > Change notes > v1->v2 > update subsystem tag in subject (davem) > repeat kfree check for peer_random and peer_hmacs (xin) > > v2->v3 > net->sctp > also free peer_chunks > > v3->v4 > fix subject tags > > v4->v5 > remove cut line > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > Reported-by: syzbot+f7e9153b037eac9b1df8@syzkaller.appspotmail.com Applied, thanks.
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index f17908f5c4f3..9b0e5b0d701a 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -2583,6 +2583,8 @@ static int sctp_process_param(struct sctp_association *asoc, case SCTP_PARAM_STATE_COOKIE: asoc->peer.cookie_len = ntohs(param.p->length) - sizeof(struct sctp_paramhdr); + if (asoc->peer.cookie) + kfree(asoc->peer.cookie); asoc->peer.cookie = kmemdup(param.cookie->body, asoc->peer.cookie_len, gfp); if (!asoc->peer.cookie) retval = 0; @@ -2647,6 +2649,8 @@ static int sctp_process_param(struct sctp_association *asoc, goto fall_through; /* Save peer's random parameter */ + if (asoc->peer.peer_random) + kfree(asoc->peer.peer_random); asoc->peer.peer_random = kmemdup(param.p, ntohs(param.p->length), gfp); if (!asoc->peer.peer_random) { @@ -2660,6 +2664,8 @@ static int sctp_process_param(struct sctp_association *asoc, goto fall_through; /* Save peer's HMAC list */ + if (asoc->peer.peer_hmacs) + kfree(asoc->peer.peer_hmacs); asoc->peer.peer_hmacs = kmemdup(param.p, ntohs(param.p->length), gfp); if (!asoc->peer.peer_hmacs) { @@ -2675,6 +2681,8 @@ static int sctp_process_param(struct sctp_association *asoc, if (!ep->auth_enable) goto fall_through; + if (asoc->peer.peer_chunks) + kfree(asoc->peer.peer_chunks); asoc->peer.peer_chunks = kmemdup(param.p, ntohs(param.p->length), gfp); if (!asoc->peer.peer_chunks)
Based on comments from Xin, even after fixes for our recent syzbot report of cookie memory leaks, its possible to get a resend of an INIT chunk which would lead to us leaking cookie memory. To ensure that we don't leak cookie memory, free any previously allocated cookie first. Change notes v1->v2 update subsystem tag in subject (davem) repeat kfree check for peer_random and peer_hmacs (xin) v2->v3 net->sctp also free peer_chunks v3->v4 fix subject tags v4->v5 remove cut line Signed-off-by: Neil Horman <nhorman@tuxdriver.com> Reported-by: syzbot+f7e9153b037eac9b1df8@syzkaller.appspotmail.com CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> CC: Xin Long <lucien.xin@gmail.com> CC: "David S. Miller" <davem@davemloft.net> CC: netdev@vger.kernel.org --- net/sctp/sm_make_chunk.c | 8 ++++++++ 1 file changed, 8 insertions(+)