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: <YtfTeJrnViCejwC4@ip-172-31-24-42.ap-northeast-1.compute.internal>
Date:   Wed, 20 Jul 2022 10:05:44 +0000
From:   Hyeonggon Yoo <42.hyeyoo@...il.com>
To:     Marco Elver <elver@...gle.com>
Cc:     Christoph Lameter <cl@...two.de>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Roman Gushchin <roman.gushchin@...ux.dev>,
        Joe Perches <joe@...ches.com>,
        Vasily Averin <vasily.averin@...ux.dev>,
        Matthew WilCox <willy@...radead.org>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 16/16] mm/sl[au]b: check if large object is valid in
 __ksize()

On Thu, Jul 14, 2022 at 12:30:09PM +0200, Marco Elver wrote:
> On Thu, 14 Jul 2022 at 11:16, Christoph Lameter <cl@...two.de> wrote:
> >
> > On Wed, 13 Jul 2022, Marco Elver wrote:
> >
> > > We shouldn't crash, so it should be WARN(), but also returning
> > > PAGE_SIZE is bad. The intuition behind returning 0 is to try and make
> > > the buggy code cause less harm to the rest of the kernel.
> > >
> > > >From [1]:
> > >
> > > > Similarly, if you are able to tell if the passed pointer is not a
> > > > valid object some other way, you can do something better - namely,
> > > > return 0. The intuition here is that the caller has a pointer to an
> > > > invalid object, and wants to use ksize() to determine its size, and
> > > > most likely access all those bytes. Arguably, at that point the kernel
> > > > is already in a degrading state. But we can try to not let things get
> > > > worse by having ksize() return 0, in the hopes that it will stop
> > > > corrupting more memory. It won't work in all cases, but should avoid
> > > > things like "s = ksize(obj); touch_all_bytes(obj, s)" where the size
> > > > bounds the memory accessed corrupting random memory.
> >
> > "in the hopes that it will stop corrupting memory"!!!???
> >
> > Do a BUG() then and definitely stop all chances of memory corruption.
> 
> Fair enough.
> 
> Well, I'd also prefer to just kill the kernel. But some people don't
> like that and want the option to continue running. So a WARN() gives
> that option, and just have to boot the kernel with panic_on_warn to
> kill it. There are other warnings in the kernel where we'd better kill
> the kernel as the chances of corrupting memory are pretty damn high if
> we hit them. And I still don't quite see why the case here is any more
> or less special.
> 
> If the situation here is exceedingly rare, let's try BUG() and see what breaks?

Let's try BUG() for both conditions and replace it with WARN() later
if kernel hit those often.

I'll update this patch in next version.

And I have no strong opinion on returning 0, but if kernel hits it a
lot, I think returning 0 would be more safe as you said.

Thanks,
Hyeonggon

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ