linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Dave Jones <davej@codemonkey.org.uk>,
	Kees Cook <keescook@chromium.org>,
	Tommi Rantala <tommi.t.rantala@nokia.com>,
	Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Laura Abbott <labbott@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Eric Biggers <ebiggers@google.com>, X86 ML <x86@kernel.org>,
	Andrew Lutomirski <luto@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: sudo x86info -a => kernel BUG at mm/usercopy.c:78!
Date: Fri, 31 Mar 2017 11:57:20 -0700	[thread overview]
Message-ID: <CALCETrWm+FkoFKN-g3p8G59PNDgE4MMR9yh7aBqz6C+TZyBgzg@mail.gmail.com> (raw)
In-Reply-To: <20170331180341.2v7iwln3gndqfmut@codemonkey.org.uk>

On Fri, Mar 31, 2017 at 11:03 AM, Dave Jones <davej@codemonkey.org.uk> wrote:
> On Fri, Mar 31, 2017 at 10:32:04AM -0700, Kees Cook wrote:
>
>  > >  > >  > > Full dmesg output here: https://pastebin.com/raw/Kur2mpZq
>  > >  > >  > >
>  > >  > >  > > [   51.418954] usercopy: kernel memory exposure attempt detected from
>  > >  > >  > > ffff880000090000 (dma-kmalloc-256) (4096 bytes)
>  > >  > >  >
>  > >  > >  > This seems like a real exposure: the copy is attempting to read 4096
>  > >  > >  > bytes from a 256 byte object.
>  > >  > >
>  > >  > > The code[1] is doing a 4k read from /dev/mem in the range 0x90000 -> 0xa0000
>  > >  > > According to arch/x86/mm/init.c:devmem_is_allowed, that's still valid..
>  > >  > >
>  > >  > > Note that the printk is using the direct mapping address. Is that what's
>  > >  > > being passed down to devmem_is_allowed now ? If so, that's probably what broke.
>  > >  >
>  > >  > So this is attempting to read physical memory 0x90000 -> 0xa0000, but
>  > >  > that's somehow resolving to a virtual address that is claimed by
>  > >  > dma-kmalloc?? I'm confused how that's happening...
>  > >
>  > > /dev/mem is using physical addresses that the kernel translates through the
>  > > direct mapping.  __check_object_size seems to think that anything passed
>  > > into it is always allocated by the kernel, but in this case, I think read_mem()
>  > > is just passing through the direct mapping to copy_to_user.
>  >
>  > How is ffff880000090000 both in the direct mapping and a slab object?
>  >
>  > It would need to pass all of these checks, and be marked as PageSlab
>  > before it could be evaluated by __check_heap_object:
>  >
>  >         if (is_vmalloc_or_module_addr(ptr))
>  >                 return NULL;
>  >
>  >         if (!virt_addr_valid(ptr))
>  >                 return NULL;
>  >
>  >         page = virt_to_head_page(ptr);
>  >
>  >         /* Check slab allocator for flags and size. */
>  >         if (PageSlab(page))
>  >                 return __check_heap_object(ptr, n, page);
>
> Looking at Tommi's dmesg output closer, it appears that he's booting in
> EFI mode (which isn't unusual these days).  I'm not sure that the EBDA
> (that x86info is trying to read) even exists under EFI, which is
> probably why the memory range is showing up as usable, and then ending
> up as a slab page, rather than being reserved by the BIOS.
>

This stuff all sucks.  Presumably the only reason that we pay
attention to the EBDA at all in EFI mode is that no one has the guts
to change it: maybe there's a firmware out there that puts something
important in the EBDA and fails to properly reserve it in the EFI
memory map.

> ...
> reserve setup_data: [mem 0x0000000000059000-0x000000000009dfff] usable
> ...
>
> If EBDA under EFI isn't a valid thing, the puzzling part is why there's
> still an EBDA pointer in lowmem. x86 people ?
>
> Longterm, I think I'm just going to gut all the ebda code from x86info,
> as it isn't really necessary.  Whether we still need to change /dev/mem
> to cope with this situation depends on whether there are other valid
> usecases.

I would like to at least consider a stricter alternative: make
/dev/mem a real whitelist.  The rules would be that, by default,
/dev/mem access is always rejected.  Kernel code could explicitly
register resources that would be permitted via /dev/mem -- each
resource would be tagged with a bit saying "devmem okay" along with
some indication of caching mode.  For example, on very recent kernels,
some crappy HP tools are busted because they try to access SMBIOS
using explicit uncached devmem accesses, but that's verboten because
the kernel accesses it with ioremap_cache().

There are really very few cases where /dev/mem is okay at all, I
think.  Maybe the EBDA is one of them.  And we could make up some hack
where devmem access to certain ranges just gets all zeros regardless
of what's actually there.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-03-31 18:57 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-30  6:44 Tommi Rantala
2017-03-30 16:45 ` Kees Cook
2017-03-30 17:20   ` Mark Rutland
2017-03-30 17:27   ` Laura Abbott
2017-03-30 17:37     ` Kees Cook
2017-03-30 17:44       ` Laura Abbott
2017-03-31  5:44         ` Tommi Rantala
2017-03-30 19:41   ` Dave Jones
2017-03-30 19:52     ` Kees Cook
2017-03-30 20:01       ` Dave Jones
2017-03-31  5:40         ` Tommi Rantala
2017-03-31  6:59           ` Tommi Rantala
2017-03-31 17:17       ` Dave Jones
2017-03-31 17:32         ` Kees Cook
2017-03-31 18:03           ` Dave Jones
2017-03-31 18:57             ` Andy Lutomirski [this message]
2017-03-31 18:26           ` Linus Torvalds
2017-03-31 19:32             ` Tommi Rantala
2017-04-04 22:37               ` Kees Cook
2017-04-04 22:55                 ` Linus Torvalds
2017-04-04 22:59                   ` Kees Cook
2017-04-05  0:22                   ` Linus Torvalds
2017-04-05 19:39                     ` Kees Cook
2017-03-31 23:58             ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALCETrWm+FkoFKN-g3p8G59PNDgE4MMR9yh7aBqz6C+TZyBgzg@mail.gmail.com \
    --to=luto@amacapital.net \
    --cc=davej@codemonkey.org.uk \
    --cc=ebiggers@google.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=labbott@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=tommi.t.rantala@nokia.com \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox