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: <20200914112037.GA2417148@chrisdown.name>
Date:   Mon, 14 Sep 2020 12:20:37 +0100
From:   Chris Down <chris@...isdown.name>
To:     Michal Hocko <mhocko@...e.com>
Cc:     Muchun Song <songmuchun@...edance.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Cgroups <cgroups@...r.kernel.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [External] Re: [PATCH] mm: memcontrol: Fix out-of-bounds on the
 buf returned by memory_stat_format

Michal Hocko writes:
>> > > Yeah, I think we should cc:stable.
>> >
>> > Is this a real problem? The buffer should contain 36 lines which makes
>> > it more than 100B per line. I strongly suspect we are not able to use
>> > that storage up.
>>
>> Before memory_stat_format() return, we should call seq_buf_putc(&s, '\0').
>> Otherwise, the return buf string has no trailing null('\0'). But users treat buf
>> as a string(and read the string oob). It is wrong. Thanks.
>
>I am not sure I follow you. vsnprintf which is used by seq_printf will
>add \0 if there is a room for that. And I argue there is a lot of room
>in the buffer so a corner case where the buffer gets full doesn't happen
>with the current code.

I don't feel very strongly either way, but in general I agree with Michal. As 
it is this feels quite perfunctory.

If you can demonstrate reading the string out of bounds in a 
userspace-exploitable way -- that is, you can demonstrate one can coerce 
memory.stat to a format where you would read out of bounds -- then we should 
obviously cc stable and keep the Fixes tag, but you have not come close to 
demonstrating this yet.

Otherwise, if you cannot provide any way to read the string out of bounds, 
because the buffer is simply way too big for this to ever happen, this is just 
a code cleanup, and neither Fixes nor stable are appropriate.

So, the question is, which is it? :-)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ