linux-kernel - Re: [PATCH] Convert filldir[64]() from __put_user() to unsafe_put

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87h84avffi.fsf@mpe.ellerman.id.au>
Date:   Tue, 15 Oct 2019 14:46:41 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] Convert filldir[64]() from __put_user() to unsafe_put_user()

Linus Torvalds <torvalds@...ux-foundation.org> writes:
> On Sun, Oct 13, 2019 at 12:59 PM Al Viro <viro@...iv.linux.org.uk> wrote:
>> Re plotting: how strongly would you object against passing the range to
>> user_access_end()?  Powerpc folks have a very close analogue of stac/clac,
>> currently buried inside their __get_user()/__put_user()/etc. - the same
>> places where x86 does, including futex.h and friends.
>>
>> And there it's even costlier than on x86.  It would obviously be nice
>> to lift it at least out of unsafe_get_user()/unsafe_put_user() and
>> move into user_access_begin()/user_access_end(); unfortunately, in
>> one subarchitecture they really want it the range on the user_access_end()
>> side as well.
>
> Hmm. I'm ok with that.
>
> Do they want the actual range, or would it prefer some kind of opaque
> cookie that user_access_begin() returns (where 0 would mean "failure"
> of course)?

The code does want the actual range, or at least the range rounded to a
segment boundary (256MB).

But it can get that already from a value it stashes in current->thread,
it was just more natural to pass the addr/size with the way the code is
currently structured.

It seems to generate slightly better code to pass addr/size vs loading
it from current->thread, but it's probably in the noise vs everything
else that's going on.

So a cookie would work fine, we could return the encoded addr/size in
the cookie and that might generate better code than loading it back from
current->thread. Equally we could just use the value in current->thread
and not have any cookie at all.

cheers