linux-kernel - Re: memory leak in sctp_process

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190529233757.GC3713@localhost.localdomain>
Date:   Wed, 29 May 2019 20:37:57 -0300
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Neil Horman <nhorman@...driver.com>
Cc:     syzbot <syzbot+f7e9153b037eac9b1df8@...kaller.appspotmail.com>,
        davem@...emloft.net, linux-kernel@...r.kernel.org,
        linux-sctp@...r.kernel.org, netdev@...r.kernel.org,
        syzkaller-bugs@...glegroups.com, vyasevich@...il.com
Subject: Re: memory leak in sctp_process_init

On Wed, May 29, 2019 at 03:07:09PM -0400, Neil Horman wrote:
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2419,9 +2419,12 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
>  	/* Copy cookie in case we need to resend COOKIE-ECHO. */
>  	cookie = asoc->peer.cookie;
>  	if (cookie) {
> +		if (asoc->peer.cookie_allocated)
> +			kfree(cookie);
>  		asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
>  		if (!asoc->peer.cookie)
>  			goto clean_up;
> +		asoc->peer.cookie_allocated=1;
>  	}
>  
>  	/* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily

What if we kmemdup directly at sctp_process_param(), as it's done for
others already? Like SCTP_PARAM_RANDOM and SCTP_PARAM_HMAC_ALGO. I
don't see a reason for SCTP_PARAM_STATE_COOKIE to be different
here. This way it would be always allocated, and ready to be kfreed.

We still need to free it after the handshake, btw.

  Marcelo