linux-kernel - Re: [RFC] mlock/stack guard interaction fixup

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: <AANLkTi=wWcAg8MdWy4O-0au3m6mH+9xWDOKVCJ_x5G_v@mail.gmail.com>
Date:	Sun, 22 Aug 2010 11:21:34 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Greg KH <gregkh@...e.de>
Cc:	Ian Campbell <ijc@...lion.org.uk>, linux-kernel@...r.kernel.org,
	stable@...nel.org, stable-review@...nel.org,
	akpm@...ux-foundation.org, alan@...rguk.ukuu.org.uk,
	Peter Zijlstra <peterz@...radead.org>,
	Jeremy Fitzhardinge <jeremy@...p.org>
Subject: Re: [RFC] mlock/stack guard interaction fixup

On Sun, Aug 22, 2010 at 10:25 AM, Greg KH <gregkh@...e.de> wrote:
>>
>> Greg, we are talking about:
>> 0e8e50e20c837eeec8323bba7dcd25fe5479194c mm: make stack guard page logic use vm_prev pointer
>> 7798330ac8114c731cfab83e634c6ecedaa233d7 mm: make the mlock() stack guard page checks stricter
>> 297c5eee372478fc32fec5fe8eed711eedb13f3d mm: make the vma list be doubly linked
>
> I must be missing something, but aren't these patches just "cleanups"
> and changing the logic here to be nicer?  Or do they fix real problems
> with the previous stack guard stuff?
>
> Is it the second one you really need here?

They're all "required" (#2 needs #1, and #3 is a fix for something
that can happen in the same circumstances that #2 makes any
difference).

Although you do need to have some really odd things going on for any
of them to make any difference. Notably, you need to do mlock or
mprotect on the stack segment, which no sane program does.

That said, the change from

    start += PAGE_SIZE;

to

    addr += PAGE_SIZE;

in __mlock_vma_pages_range() (in #3) is a bugfix even for the normal
mlockall() case. Not that anybody will realistically care about that
either: the failure case just doesn't really realistically ever matter
(it expands the stack which the code tries to avoid, and possibly
forgets to mlock the bottom of the stack).

So I wouldn't call them high priority. Ian is doing something _really_
odd. Doing hypercalls from user space on stuff that is on the stack,
rather than just copying it to some stable area is dodgy. And I
guarantee that doing the crazy mlock dance is slower than the copy, so
it's complex, fragile, _and_ slow.

                            Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/