All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Anderson <anderson@redhat.com>
To: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Cc: mingo@kernel.org, andi@firstfloor.org, keescook@chromium.org
Subject: BUG: /proc/kcore does not export direct-mapped memory on arm64 (and presumably some other architectures)
Date: Thu, 26 Apr 2018 15:31:26 -0400 (EDT)	[thread overview]
Message-ID: <823082096.24861749.1524771086176.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <981100282.24860394.1524770798522.JavaMail.zimbra@redhat.com>


While testing /proc/kcore as the live memory source for the crash utility,
it fails on arm64.  The failure on arm64 occurs because only the 
vmalloc/module space segments are exported in PT_LOAD segments, 
and it's missing all of the PT_LOAD segments for the generic 
unity-mapped regions of physical memory, as well as their associated
vmemmap sections.
  
The mapping of unity-mapped RAM segments in fs/proc/kcore.c is 
architecture-neutral, and after debugging it, I found this as the
problem.  For each chunk of physical memory, kcore_update_ram()
calls walk_system_ram_range(), passing kclist_add_private() as a 
callback function to add the chunk to the kclist, and eventually 
leading to the creation of a PT_LOAD segment.
  
kclist_add_private() does some verification of the memory region,
but this one below is bogus for arm64:
  
    static int
    kclist_add_private(unsigned long pfn, unsigned long nr_pages, void *arg)
    {
    ... [ cut ] ...
            ent->addr = (unsigned long)__va((pfn << PAGE_SHIFT));
    ... [ cut ] ...
  
            /* Sanity check: Can happen in 32bit arch...maybe */
            if (ent->addr < (unsigned long) __va(0))
                    goto free_out;
  
And that's because __va(0) is a bogus check for arm64.  It is checking
whether the ent->addr value is less than the lowest possible unity-mapped
address.  But "0" should not be used as a physical address on arm64; the 
lowest legitimate physical address for this __va() check would be the arm64 
PHYS_OFFSET, or memstart_addr:
  
Here's the arm64 __va() and PHYS_OFFSET:
  
  #define __va(x) ((void *)__phys_to_virt((phys_addr_t)(x)))
  #define __phys_to_virt(x)       ((unsigned long)((x) - PHYS_OFFSET) | PAGE_OFFSET)
  
  extern s64                      memstart_addr;
  /* PHYS_OFFSET - the physical address of the start of memory. */
  #define PHYS_OFFSET             ({ VM_BUG_ON(memstart_addr & 1); memstart_addr; })
  
If PHYS_OFFSET/memstart_addr is anything other than 0 (it is 0x4000000000 on my 
test system), the __va(0) calculation goes negative and creates a bogus, very 
large, virtual address.  And since the ent->addr virtual address is less than 
bogus __va(0) address, the test fails, and the memory chunk is rejected. 
  
Looking at the kernel sources, it seems that this would affect other
architectures as well, i.e., the ones whose __va() is not a simple
addition of the physical address with PAGE_OFFSET.
  
Anyway, I don't know what the best approach for an architecture-neutral
fix would be in this case.  So I figured I'd throw it out to you guys for
some ideas.

Dave Anderson

       reply	other threads:[~2018-04-26 19:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <981100282.24860394.1524770798522.JavaMail.zimbra@redhat.com>
2018-04-26 19:31 ` Dave Anderson [this message]
2018-04-26 21:16   ` BUG: /proc/kcore does not export direct-mapped memory on arm64 (and presumably some other architectures) Kees Cook
2018-04-28  0:58     ` Laura Abbott
2018-04-30 14:03       ` Dave Anderson
2018-05-01 14:45         ` Dave Anderson
2018-05-01 20:11           ` [PATCH] proc/kcore: Don't bounds check against address 0 Laura Abbott
2018-05-01 20:11             ` Laura Abbott
2018-05-01 21:46             ` Andrew Morton
2018-05-01 21:46               ` Andrew Morton
2018-05-01 22:26               ` Laura Abbott
2018-05-01 22:26                 ` Laura Abbott

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=823082096.24861749.1524771086176.JavaMail.zimbra@redhat.com \
    --to=anderson@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.