From: Michael Ellerman <mpe@ellerman.id.au>
To: Christophe Leroy <christophe.leroy@c-s.fr>, linuxppc-dev@ozlabs.org
Cc: npiggin@gmail.com
Subject: Re: [PATCH v5 05/10] powerpc: Add a framework for Kernel Userspace Access Protection
Date: Fri, 22 Mar 2019 11:35:21 +1100 [thread overview]
Message-ID: <87a7hnkapi.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: <7fad467c-1e63-02c8-636e-7f8ab456f76c@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 20/03/2019 à 13:57, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>> Le 08/03/2019 à 02:16, Michael Ellerman a écrit :
>>>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>>>>
>>>> This patch implements a framework for Kernel Userspace Access
>>>> Protection.
>>>>
>>>> Then subarches will have the possibility to provide their own
>>>> implementation by providing setup_kuap() and
>>>> allow/prevent_user_access().
>>>>
>>>> Some platforms will need to know the area accessed and whether it is
>>>> accessed from read, write or both. Therefore source, destination and
>>>> size and handed over to the two functions.
>>>>
>>>> mpe: Rename to allow/prevent rather than unlock/lock, and add
>>>> read/write wrappers. Drop the 32-bit code for now until we have an
>>>> implementation for it. Add kuap to pt_regs for 64-bit as well as
>>>> 32-bit. Don't split strings, use pr_crit_ratelimited().
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> Signed-off-by: Russell Currey <ruscur@russell.cc>
>>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>>> ---
>>>> v5: Futex ops need read/write so use allow_user_acccess() there.
>>>> Use #ifdef CONFIG_PPC64 in kup.h to fix build errors.
>>>> Allow subarch to override allow_read/write_from/to_user().
>>>
>>> Those little helpers that will just call allow_user_access() when
>>> distinct read/write handling is not performed looks overkill to me.
>>>
>>> Can't the subarch do it by itself based on the nullity of from/to ?
>>>
>>> static inline void allow_user_access(void __user *to, const void __user
>>> *from,
>>> unsigned long size)
>>> {
>>> if (to & from)
>>> set_kuap(0);
>>> else if (to)
>>> set_kuap(AMR_KUAP_BLOCK_READ);
>>> else if (from)
>>> set_kuap(AMR_KUAP_BLOCK_WRITE);
>>> }
>>
>> You could implement it that way, but it reads better at the call sites
>> if we have:
>>
>> allow_write_to_user(uaddr, sizeof(*uaddr));
>> vs:
>> allow_user_access(uaddr, NULL, sizeof(*uaddr));
>>
>> So I'm inclined to keep them. It should all end up inlined and generate
>> the same code at the end of the day.
>>
>
> I was not suggesting to completly remove allow_write_to_user(), I fully
> agree that it reads better at the call sites.
>
> I was just thinking that allow_write_to_user() could remain generic and
> call the subarch specific allow_user_access() instead of making multiple
> subarch's allow_write_to_user()
Yep OK I see what you mean.
Your suggestion above should work, and involves the least amount of
ifdefs and so on.
I'll try and get time to post a v6.
cheers
next prev parent reply other threads:[~2019-03-22 0:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-08 1:16 [PATCH v5 01/10] powerpc/powernv/idle: Restore IAMR after idle Michael Ellerman
2019-03-08 1:16 ` [PATCH v5 02/10] powerpc/powernv/idle: Restore AMR/UAMOR/AMOR " Michael Ellerman
2019-03-13 8:16 ` Akshay Adiga
2019-03-20 12:58 ` Michael Ellerman
2019-03-08 1:16 ` [PATCH v5 03/10] powerpc: Add framework for Kernel Userspace Protection Michael Ellerman
2019-03-08 1:16 ` [PATCH v5 04/10] powerpc: Add skeleton for Kernel Userspace Execution Prevention Michael Ellerman
2019-03-08 1:16 ` [PATCH v5 05/10] powerpc: Add a framework for Kernel Userspace Access Protection Michael Ellerman
2019-03-08 8:26 ` Christophe Leroy
2019-03-20 12:57 ` Michael Ellerman
2019-03-20 13:04 ` Christophe Leroy
2019-03-21 10:21 ` Christophe Leroy
2019-03-22 0:35 ` Michael Ellerman [this message]
2019-03-11 9:12 ` Christophe Leroy
2019-03-08 1:16 ` [PATCH v5 06/10] powerpc/64: Setup KUP on secondary CPUs Michael Ellerman
2019-03-08 1:16 ` [PATCH v5 07/10] powerpc/mm/radix: Use KUEP API for Radix MMU Michael Ellerman
2019-03-08 1:16 ` [PATCH v5 08/10] powerpc/lib: Refactor __patch_instruction() to use __put_user_asm() Michael Ellerman
2019-03-08 8:32 ` Christophe Leroy
2019-03-08 1:16 ` [PATCH v5 09/10] powerpc/64s: Implement KUAP for Radix MMU Michael Ellerman
2019-03-08 8:48 ` Christophe Leroy
2019-04-17 13:39 ` Michael Ellerman
2019-03-08 1:16 ` [PATCH v5 10/10] powerpc/mm: Detect bad KUAP faults Michael Ellerman
2019-03-08 8:53 ` Christophe Leroy
2019-03-09 12:49 ` christophe leroy
2019-04-17 13:17 ` Michael Ellerman
2019-04-17 13:12 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a7hnkapi.fsf@concordia.ellerman.id.au \
--to=mpe@ellerman.id.au \
--cc=christophe.leroy@c-s.fr \
--cc=linuxppc-dev@ozlabs.org \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).