[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 6 Jun 2017 21:08:44 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: igor.stoppa@...wei.com
Cc: keescook@...omium.org, mhocko@...nel.org, jmorris@...ei.org,
paul@...l-moore.com, sds@...ho.nsa.gov, casey@...aufler-ca.com,
hch@...radead.org, labbott@...hat.com, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH 2/5] Protectable Memory Allocator
Igor Stoppa wrote:
> >> +struct pmalloc_node {
> >> + struct hlist_node nodes_list;
> >> + atomic_t used_words;
> >> + unsigned int total_words;
> >> + __PMALLOC_ALIGNED align_t data[];
> >> +};
> >
> > Is this __PMALLOC_ALIGNED needed? Why not use "long" and "BITS_PER_LONG" ?
>
> In an earlier version I actually asked the same question.
> It is currently there because I just don't know enough about various
> architectures. The idea of having "align_t" was that it could be tied
> into what is the most desirable alignment for each architecture.
> But I'm actually looking for advise on this.
I think that let the compiler use natural alignment is OK.
> > You need to check for node != NULL before dereference it.
>
> So, if I understood correctly, there shouldn't be a case where node is
> NULL, right?
> Unless it has been tampered/damaged. Is that what you mean?
I meant to say
+ node = __pmalloc_create_node(req_words);
// this location.
+ starting_word = atomic_fetch_add(req_words, &node->used_words);
> >> +const char *__pmalloc_check_object(const void *ptr, unsigned long n)
> >> +{
> >> + unsigned long p;
> >> +
> >> + p = (unsigned long)ptr;
> >> + n += (unsigned long)ptr;
> >> + for (; (PAGE_MASK & p) <= (PAGE_MASK & n); p += PAGE_SIZE) {
> >> + if (is_vmalloc_addr((void *)p)) {
> >> + struct page *page;
> >> +
> >> + page = vmalloc_to_page((void *)p);
> >> + if (!(page && PagePmalloc(page)))
> >> + return msg;
> >> + }
> >> + }
> >> + return NULL;
> >> +}
> >
> > I feel that n is off-by-one if (ptr + n) % PAGE_SIZE == 0
> > according to check_page_span().
>
> It seems to work. If I am missing your point, could you please
> use the same format of the example I made, to explain me?
If ptr == NULL and n == PAGE_SIZE so that (ptr + n) % PAGE_SIZE == 0,
this loop will access two pages (one page containing p == 0 and another
page containing p == PAGE_SIZE) when this loop should access only one
page containing p == 0. When checking n bytes, it's range is 0 to n - 1.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
