linux-kernel - Re: [RFC 2/3] vmalloc: Support grouped page allocations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <971aae01-32a0-3f45-1810-010e3295b1c4@intel.com>
Date:   Mon, 5 Apr 2021 14:01:58 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Rick Edgecombe <rick.p.edgecombe@...el.com>,
        akpm@...ux-foundation.org, linux-mm@...ck.org, bpf@...r.kernel.org,
        dave.hansen@...ux.intel.com, peterz@...radead.org, luto@...nel.org,
        jeyu@...nel.org
Cc:     linux-kernel@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        andrii@...nel.org, hch@...radead.org, x86@...nel.org
Subject: Re: [RFC 2/3] vmalloc: Support grouped page allocations

On 4/5/21 1:37 PM, Rick Edgecombe wrote:
> +static void __dispose_pages(struct list_head *head)
> +{
> +	struct list_head *cur, *next;
> +
> +	list_for_each_safe(cur, next, head) {
> +		list_del(cur);
> +
> +		/* The list head is stored at the start of the page */
> +		free_page((unsigned long)cur);
> +	}
> +}

This is interesting.

While the page is in the allocator, you're using the page contents
themselves to store the list_head.  It took me a minute to figure out
what you were doing here because: "start of the page" is a bit
ambiguous.  It could mean:

 * the first 16 bytes in 'struct page'
or
 * the first 16 bytes in the page itself, aka *page_address(page)

The fact that this doesn't work on higmem systems makes this an OK thing
to do, but it is a bit weird.  It's also doubly susceptible to bugs
where there's a page_to_virt() or virt_to_page() screwup.

I was *hoping* there was still sufficient space in 'struct page' for
this second list_head in addition to page->lru.  I think there *should*
be.  That would at least make this allocator a bit more "normal" in not
caring about page contents while the page is free in the allocator.  If
you were able to do that you could do things like kmemcheck or page
alloc debugging while the page is in the allocator.

Anyway, I think I'd prefer that you *try* to use 'struct page' alone.
But, if that doesn't work out, please comment the snot out of this thing
because it _is_ weird.