* [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key
@ 2020-06-27 7:01 Aneesh Kumar K.V
2020-06-27 7:01 ` [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled() Aneesh Kumar K.V
2020-06-30 12:24 ` [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Michael Ellerman
0 siblings, 2 replies; 4+ messages in thread
From: Aneesh Kumar K.V @ 2020-06-27 7:01 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, Jan Stancek
pkey_access_permitted() should not check for pkey is available in UAMOR or not.
The kernel needs to do that check only while allocating keys. This also makes
sure execute_only_key which is marked as non-manageable via UAMOR gives the
right access check return w.r.t pkey_access_permitted().
This fix the page fault loop when using PROT_EXEC as below
addr = mmap(0, page_sz, PROT_EXEC, MAP_FILE | MAP_PRIVATE, fildes, 0);
x = *addr);
Fixes: c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")
Reported-by: Jan Stancek <jstancek@redhat•com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux•ibm.com>
---
arch/powerpc/mm/book3s64/pkeys.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 1199fc2bfaec..ca5fcb4bff32 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -353,9 +353,6 @@ static bool pkey_access_permitted(int pkey, bool write, bool execute)
int pkey_shift;
u64 amr;
- if (!is_pkey_enabled(pkey))
- return true;
-
pkey_shift = pkeyshift(pkey);
if (execute && !(read_iamr() & (IAMR_EX_BIT << pkey_shift)))
return true;
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled()
2020-06-27 7:01 [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Aneesh Kumar K.V
@ 2020-06-27 7:01 ` Aneesh Kumar K.V
2020-06-30 12:21 ` Michael Ellerman
2020-06-30 12:24 ` [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Michael Ellerman
1 sibling, 1 reply; 4+ messages in thread
From: Aneesh Kumar K.V @ 2020-06-27 7:01 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
Rename is_pkey_enabled() to is_pkey_masked() to better indicates that
this check is to make sure the key is available for userspace usage. For it to
be made available both the bits in UAMOR should be set to 1 (0b11).
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux•ibm.com>
---
arch/powerpc/mm/book3s64/pkeys.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index ca5fcb4bff32..70d760ade922 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -206,18 +206,16 @@ static inline void write_uamor(u64 value)
mtspr(SPRN_UAMOR, value);
}
-static bool is_pkey_enabled(int pkey)
+static bool is_pkey_masked(int pkey)
{
u64 uamor = read_uamor();
u64 pkey_bits = 0x3ul << pkeyshift(pkey);
u64 uamor_pkey_bits = (uamor & pkey_bits);
/*
- * Both the bits in UAMOR corresponding to the key should be set or
- * reset.
+ * Both the bits in UAMOR corresponding to the key should be set
*/
- WARN_ON(uamor_pkey_bits && (uamor_pkey_bits != pkey_bits));
- return !!(uamor_pkey_bits);
+ return (uamor_pkey_bits != pkey_bits);
}
static inline void init_amr(int pkey, u8 init_bits)
@@ -246,7 +244,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
u64 new_amr_bits = 0x0ul;
u64 new_iamr_bits = 0x0ul;
- if (!is_pkey_enabled(pkey))
+ if (is_pkey_masked(pkey))
return -EINVAL;
if (init_val & PKEY_DISABLE_EXECUTE) {
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled()
2020-06-27 7:01 ` [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled() Aneesh Kumar K.V
@ 2020-06-30 12:21 ` Michael Ellerman
0 siblings, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2020-06-30 12:21 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V
"Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com> writes:
> Rename is_pkey_enabled() to is_pkey_masked() to better indicates that
> this check is to make sure the key is available for userspace usage.
I don't think the new name makes that any clearer. Unless you know that
"masked" means not "available for userspace".
It's also not clear if masked means 00 or 11.
Now that there's only one caller why not just fold it in, that way it
doesn't need a name at all.
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index ca5fcb4bff32..70d760ade922 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -206,18 +206,16 @@ static inline void write_uamor(u64 value)
> mtspr(SPRN_UAMOR, value);
> }
>
> -static bool is_pkey_enabled(int pkey)
> +static bool is_pkey_masked(int pkey)
> {
> u64 uamor = read_uamor();
> u64 pkey_bits = 0x3ul << pkeyshift(pkey);
> u64 uamor_pkey_bits = (uamor & pkey_bits);
>
> /*
> - * Both the bits in UAMOR corresponding to the key should be set or
> - * reset.
> + * Both the bits in UAMOR corresponding to the key should be set
> */
> - WARN_ON(uamor_pkey_bits && (uamor_pkey_bits != pkey_bits));
> - return !!(uamor_pkey_bits);
> + return (uamor_pkey_bits != pkey_bits);
> }
>
> static inline void init_amr(int pkey, u8 init_bits)
> @@ -246,7 +244,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> u64 new_amr_bits = 0x0ul;
> u64 new_iamr_bits = 0x0ul;
>
> - if (!is_pkey_enabled(pkey))
> + if (is_pkey_masked(pkey))
> return -EINVAL;
eg:
u64 pkey_bits = 0x3ul << pkeyshift(pkey);
if ((read_uamor() & pkey_bits) != pkey_bits)
return -EINVAL;
>
> if (init_val & PKEY_DISABLE_EXECUTE) {
cheers
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key
2020-06-27 7:01 [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Aneesh Kumar K.V
2020-06-27 7:01 ` [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled() Aneesh Kumar K.V
@ 2020-06-30 12:24 ` Michael Ellerman
1 sibling, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2020-06-30 12:24 UTC (permalink / raw)
To: linuxppc-dev, mpe, Aneesh Kumar K.V; +Cc: Jan Stancek
On Sat, 27 Jun 2020 12:31:46 +0530, Aneesh Kumar K.V wrote:
> pkey_access_permitted() should not check for pkey is available in UAMOR or not.
> The kernel needs to do that check only while allocating keys. This also makes
> sure execute_only_key which is marked as non-manageable via UAMOR gives the
> right access check return w.r.t pkey_access_permitted().
>
> This fix the page fault loop when using PROT_EXEC as below
>
> [...]
Patch 1 applied to powerpc/fixes.
[1/2] powerpc/mm/pkeys: Make pkey access check work on execute_only_key
https://git.kernel.org/powerpc/c/19ab500edb5d6020010caba48ce3b4ce4182ab63
cheers
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-30 12:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-27 7:01 [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Aneesh Kumar K.V
2020-06-27 7:01 ` [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled() Aneesh Kumar K.V
2020-06-30 12:21 ` Michael Ellerman
2020-06-30 12:24 ` [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox