linux-kernel - Re: Possible memory leak in unxz()

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: <daf2f87a-155c-0836-cf82-0b409b860d6d@infradead.org>
Date:   Sun, 3 May 2020 20:58:02 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Dongyang Zhan <zdyzztq@...il.com>, linux-kernel@...r.kernel.org,
        Lasse Collin <lasse.collin@...aani.org>
Subject: Re: Possible memory leak in unxz()

On 5/3/20 12:23 AM, Dongyang Zhan wrote:
> Hi,
> 
> I am a security researcher, my name is Dongyang Zhan. I found a potential bug.
> 
> I hope you can help me to confirm it.
> 
> Thank you.
> 
> Possible memory leak in Linux 4.10.17. The function unxz() in

It would be more helpful if you could focus on more recent/current
source code.  If someone makes a patch for current source code and
it needs to be backported to older kernels, then that would [normally]
happen.

> /lib/decompress_unxz.c forgets to free the pointer 'in', when  the
> statement if (fill == NULL && flush == NULL) is true.

Adding xz contributor to email.

I think that you are correct. (I am looking at 5.7-rc4.)

However, I don't see any calls to __decompress() in the
Linux kernel that pass a first argument of NULL, so while
the code in unxz() could be fixed, we aren't currently leaking
any memory AFAICT.


> Source code and comments:
> 
> if (in == NULL) {
> must_free_in = true;
> in = malloc(XZ_IOBUF_SIZE);
> if (in == NULL)
> goto error_alloc_in;
> }
> 
> b.in = in;
> b.in_pos = 0;
> b.in_size = in_size;
> b.out_pos = 0;
> 
> if (fill == NULL && flush == NULL) {
> ret = xz_dec_run(s, &b); // When this statement is true, it will jumps
> to the switch statement. But the allocated 'in' is not freed before
> return.
> } else {
> .....
> }
> .....
> switch (ret) {
> case XZ_STREAM_END:
> return 0;
> 
> case XZ_MEM_ERROR:
> /* This can occur only in multi-call mode. */
> error("XZ decompressor ran out of memory");
> break;
> 
> case XZ_FORMAT_ERROR:
> error("Input is not in the XZ format (wrong magic bytes)");
> break;
> 
> case XZ_OPTIONS_ERROR:
> error("Input was encoded with settings that are not "
> "supported by this XZ decoder");
> break;
> 
> case XZ_DATA_ERROR:
> case XZ_BUF_ERROR:
> error("XZ-compressed data is corrupt");
> break;
> 
> default:
> error("Bug in the XZ decompressor");
> break;
> }
> 
> return -1;
> ....

thanks.
-- 
~Randy