MIME-Version: 1.0
References: <CAMuHMdWhx0-kspDyLOgTp+9udiBpkMHULSGyMg9uPPV16=FWQg@mail.gmail.com>
 <20190319170911.GB202956@gmail.com> <20190320185719.GB180195@gmail.com>
 <CAGXu5jL-5C1ZGZ1JgaNFqK-z1Pp1N1X8gD3_ysKTCa=omD+_VQ@mail.gmail.com>
 <20190321175122.GA1587@sol.localdomain> <20190410031734.GB7140@sol.localdomain>
In-Reply-To: <20190410031734.GB7140@sol.localdomain>
From:   Kees Cook <keescook@chromium.org>
Date:   Wed, 10 Apr 2019 11:30:57 -0700
Message-ID: <CAGXu5j+SZ2+Zkc=Vp5CXQUHhiR9f_OOnca684JTRW3T0yXdaNQ@mail.gmail.com>
Subject: Re: crypto: Kernel memory overwrite attempt detected to spans
 multiple pages
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     Geert Uytterhoeven <geert@linux-m68k.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        linux-security-module <linux-security-module@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Tue, Apr 9, 2019 at 8:17 PM Eric Biggers <ebiggers@kernel.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@kernel.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.

-- 
Kees Cook