linux-kernel - Re: problem starting /sbin/init (32-bit 5.3-rc8)

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]

Date:   Fri, 13 Sep 2019 05:29:14 -0700
From:   Matthew Wilcox <willy@...radead.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Randy Dunlap <rdunlap@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>, X86 ML <x86@...nel.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Subject: Re: problem starting /sbin/init (32-bit 5.3-rc8)

On Thu, Sep 12, 2019 at 06:46:04PM -0700, Kees Cook wrote:
> This combination appears to be bugged since the original introduction
> of hardened usercopy in v4.8. Is this an untested combination until
> now? (I don't usually do tests with CONFIG_DEBUG_VIRTUAL, but I guess
> I will from now on!)

Tricky one because it is only going to trip when someone actually does
this with a highmem page, so if you have a small machine (eg <512MB)
running a 32-bit kernel, you won't hit it.

> Is kmap somewhere "unexpected" in this case? Ah-ha, yes, it seems it is.
> There is even a helper to do the "right" thing as virt_to_page(). This
> seems to be used very rarely in the kernel... is there a page type for
> kmap pages? This seems like a hack, but it fixes it:

I think this is actually the right thing to do.  It'd be better if we had
a kmap_to_head_page(), but we don't.

> @@ -227,7 +228,7 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
>  	if (!virt_addr_valid(ptr))
>  		return;
>  
> -	page = virt_to_head_page(ptr);
> +	page = compound_head(kmap_to_page((void *)ptr));
>  
>  	if (PageSlab(page)) {
>  		/* Check slab allocator for flags and size. */
> 
> 
> What's the right way to "ignore" the kmap range? (i.e. it's not Slab, so
> ignore it here: I can't find a page type nor a "is this kmap?" helper...)

I don't think we want it to be _ignored_ ... if an attempted copy crosses
outside this page boundary, we want it stopped.  So I think this patch
is as good as it can be.