From: Jerome Glisse <jglisse@redhat.com>
To: Igor Stoppa <igor.stoppa@huawei.com>
Cc: Michal Hocko <mhocko@kernel.org>, Linux-MM <linux-mm@kvack.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-security-module@vger.kernel.org,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>,
Kees Cook <keescook@google.com>
Subject: Re: [RFC] Tagging of vmalloc pages for supporting the pmalloc allocator
Date: Tue, 8 Aug 2017 19:15:36 -0400 [thread overview]
Message-ID: <20170808231535.GA20840@redhat.com> (raw)
In-Reply-To: <c06fdd1a-fb18-8e17-b4fb-ea73ccd93f90@huawei.com>
On Tue, Aug 08, 2017 at 03:59:36PM +0300, Igor Stoppa wrote:
> On 07/08/17 22:12, Jerome Glisse wrote:
> > On Mon, Aug 07, 2017 at 05:13:00PM +0300, Igor Stoppa wrote:
>
> [...]
>
> >> I have an updated version of the old proposal:
> >>
> >> * put a magic number in the private field, during initialization of
> >> pmalloc pages
> >>
> >> * during hardened usercopy verification, when I have to assess if a page
> >> is of pmalloc type, compare the private field against the magic number
> >>
> >> * if and only if the private field matches the magic number, then invoke
> >> find_vm_area(), so that the slowness affects only a possibly limited
> >> amount of false positives.
> >
> > This all sounds good to me.
>
> ok, I still have one doubt wrt defining the flag.
> Where should I do it?
>
> vmalloc.h has the following:
>
> /* bits in flags of vmalloc's vm_struct below */
> #define VM_IOREMAP 0x00000001 /* ioremap() and friends
> */
> #define VM_ALLOC 0x00000002 /* vmalloc() */
> #define VM_MAP 0x00000004 /* vmap()ed pages */
> #define VM_USERMAP 0x00000008 /* suitable for
> remap_vmalloc_range
> */
> #define VM_UNINITIALIZED 0x00000020 /* vm_struct is not
> fully initialized */
> #define VM_NO_GUARD 0x00000040 /* don't add guard page
> */
> #define VM_KASAN 0x00000080 /* has allocated kasan
> shadow memory */
> /* bits [20..32] reserved for arch specific ioremap internals */
>
>
>
> I am tempted to add
>
> #define VM_PMALLOC 0x00000100
>
> But would it be acceptable, to mention pmalloc into vmalloc?
>
> Should I name it VM_PRIVATE bit, instead?
>
> Using VM_PRIVATE would avoid contaminating vmalloc with something that
> depends on it (like VM_PMALLOC would do).
>
> But using VM_PRIVATE will likely add tracking issues, if someone else
> wants to use the same bit and it's not clear who is the user, if any.
VM_PMALLOC sounds fine to me also adding a comment there pointing to
pmalloc documentation would be a good thing to do. The above are flags
that are use only inside vmalloc context and so there is no issue
here of conflicting with other potential user.
>
> Unless it's acceptable to check the private field in the page struct.
> It would bear the pmalloc magic number.
I thought you wanted to do:
check struct page mapping field
check vmap->flags for VM_PMALLOC
bool is_pmalloc(unsigned long addr)
{
struct page *page;
struct vm_struct *vm_struct;
if (!is_vmalloc_addr(addr))
return false;
page = vmalloc_to_page(addr);
if (!page)
return false;
if (page->mapping != pmalloc_magic_key)
return false;
vm_struct = find_vm_area(addr);
if (!vm_struct)
return false;
return vm_struct->flags & VM_PMALLOC;
}
Did you change your plan ?
>
> I'm thinking to use a pointer to one of pmalloc data items, as signature.
What ever is easier for you. Note that dereferencing such pointer before
asserting this is really a pmalloc page would be hazardous.
Jérôme
next prev parent reply other threads:[~2017-08-08 23:15 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-02 15:14 [RFC] Tagging of vmalloc pages for supporting the pmalloc allocator Igor Stoppa
2017-08-02 17:08 ` Jerome Glisse
2017-08-03 10:11 ` Igor Stoppa
2017-08-03 11:48 ` Michal Hocko
2017-08-03 12:20 ` Igor Stoppa
2017-08-03 13:55 ` Michal Hocko
2017-08-03 14:41 ` Igor Stoppa
2017-08-03 14:47 ` Jerome Glisse
2017-08-03 15:06 ` Igor Stoppa
2017-08-03 15:15 ` Michal Hocko
2017-08-04 8:02 ` Igor Stoppa
2017-08-04 8:12 ` Michal Hocko
2017-08-07 11:26 ` Igor Stoppa
2017-08-07 11:34 ` Michal Hocko
2017-08-07 13:31 ` Jerome Glisse
2017-08-07 14:13 ` Igor Stoppa
2017-08-07 19:12 ` Jerome Glisse
2017-08-08 12:59 ` Igor Stoppa
2017-08-08 23:15 ` Jerome Glisse [this message]
2017-08-09 7:27 ` Igor Stoppa
2017-08-10 7:14 ` Michal Hocko
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=20170808231535.GA20840@redhat.com \
--to=jglisse@redhat.com \
--cc=igor.stoppa@huawei.com \
--cc=keescook@google.com \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mhocko@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;
as well as URLs for NNTP newsgroup(s).