Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: Possible memory leak in unxz()
To:     Dongyang Zhan <zdyzztq@gmail.com>, linux-kernel@vger.kernel.org,
        Lasse Collin <lasse.collin@tukaani.org>
References: <CAFSR4csKYu95qak02h_sAH6Rpa13PUtHUZ+7Z7Vd7tmBQCNaqg@mail.gmail.com>
From:   Randy Dunlap <rdunlap@infradead.org>
Message-ID: <daf2f87a-155c-0836-cf82-0b409b860d6d@infradead.org>
Date:   Sun, 3 May 2020 20:58:02 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <CAFSR4csKYu95qak02h_sAH6Rpa13PUtHUZ+7Z7Vd7tmBQCNaqg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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