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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sun, 10 May 2020 22:15:07 +0300
From:   Lasse Collin <lasse.collin@...aani.org>
To:     Randy Dunlap <rdunlap@...radead.org>,
        Dongyang Zhan <zdyzztq@...il.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: Possible memory leak in unxz()

> On 5/3/20 12:23 AM, Dongyang Zhan wrote:
> > I am a security researcher, my name is Dongyang Zhan. I found a
> > potential bug.

Thank you for looking for bugs!

On 2020-05-03 Randy Dunlap wrote:
> On 5/3/20 12:23 AM, Dongyang Zhan wrote:
> > /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.

The supposedly leaked memory is allocated only when in == NULL, and
it's not freed when fill == NULL && flush == NULL. However, "in" and
"fill" must never be NULL at the same time if the caller is following
the decompress_fn API defined in include/linux/decompress/generic.h. So
there is no leak.

Some implementations of the API explicitly check for input == NULL &&
fill == NULL while some don't. E.g. decompress_unlz4.c checks it but in
addition it seems to also reject the case where input != NULL && fill
!= NULL. Perhaps that combination isn't used anywhere but it is allowed
by the API ("input" would be a temporary buffer for "fill"). I think
the decompress_fn API is more complex than it looks at glance.

-- 
Lasse Collin  |  IRC: Larhzu @ IRCnet & Freenode

Powered by blists - more mailing lists