[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190410190729.GA120258@gmail.com>
Date: Wed, 10 Apr 2019 12:07:31 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>,
Herbert Xu <herbert@...dor.apana.org.au>,
linux-security-module <linux-security-module@...r.kernel.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: crypto: Kernel memory overwrite attempt detected to spans
multiple pages
On Wed, Apr 10, 2019 at 11:30:57AM -0700, Kees Cook wrote:
> On Tue, Apr 9, 2019 at 8:17 PM Eric Biggers <ebiggers@...nel.org> wrote:
> >
> > On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> > > On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > > > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@...nel.org> wrote:
> > > > >
> > > > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > > > When running the sha1-asm crypto selftest on arm with
> > > > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > > > >
> > > > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > > > multiple pages (offset 0, size 42)!
> > > > > >
> > > > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > > > buffer that was allocated with __get_free_pages():
> > > > > >
> > > > > > __get_free_pages(GFP_KERNEL, 1)
> > > > > >
> > > > > > ... where 1 means an order-1 allocation.
> > > > > >
> > > > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > > > >
> > > > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > > > with ITER_KVEC.
> > > > > >
> > > > > > - Eric
> > > > >
> > > > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > > > boundary when they seem to be fine?
> > > >
> > > > This is due to missing the compound page marking, if I remember
> > > > correctly. However, I tend to leave the pagespan test disabled: it
> > > > really isn't ready for production use -- there are a lot of missing
> > > > annotations still.
> > > >
> > >
> > > So do I need to add __GFP_COMP? Is there any actual reason to do so?
> > > Why does hardened usercopy check for it?
> > >
> > > - Eric
> >
> > Hi Kees, any answer to this question?
>
> Hi! Sorry, this got lost in my inbox. Yes, if you can add __GFP_COMP,
> that would fix this case. No one has had time lately to track down all
> these cases, but avoiding adding new ones would be wonderful. :)
>
> It's in there because it's a state I'd like to get to in the kernel,
> but it'll require a lot more work to get there.
>
That didn't answer my question. My question is what is the purpose of this? If
there was actual buffer overflow when __GFP_COMP isn't specified that would make
perfect sense, but AFAICS there isn't. So why does hardened usercopy consider
it broken when __GFP_COMP isn't specified?
- Eric
Powered by blists - more mailing lists