Re: memory leak in sctp_process_init

[Marcelo Ricardo Leitner]

On Thu, May 30, 2019 at 03:56:34PM -0400, Neil Horman wrote:
> On Thu, May 30, 2019 at 12:17:05PM -0300, Marcelo Ricardo Leitner wrote:
...
> > --- a/net/sctp/sm_sideeffect.c
> > +++ b/net/sctp/sm_sideeffect.c
> > @@ -898,6 +898,11 @@ static void sctp_cmd_new_state(struct sctp_cmd_seq *cmds,
> >  						asoc->rto_initial;
> >  	}
> >  
> > +	if (sctp_state(asoc, ESTABLISHED)) {
> > +		kfree(asoc->peer.cookie);
> > +		asoc->peer.cookie = NULL;
> > +	}
> > +
> Not sure I follow why this is needed.  It doesn't hurt anything of course, but
> if we're freeing in sctp_association_free, we don't need to duplicate the
> operation here, do we?

This one would be to avoid storing the cookie throughout the entire
association lifetime, as the cookie is only needed during the
handshake.
While the free in sctp_association_free will handle the freeing in
case the association never enters established state.

> >  	if (sctp_state(asoc, ESTABLISHED) ||
> >  	    sctp_state(asoc, CLOSED) ||
> >  	    sctp_state(asoc, SHUTDOWN_RECEIVED)) {
> > 
> > Also untested, just sharing the idea.
> > 
> >   Marcelo
> >