Le 11/06/2022 à 01:35, ira.weiny@intel.com a écrit :
From: Ira Weiny ira.weiny@intel.com
x86 is missing a hardware check for pkey support in pkey_free(). While the net result is the same (-EINVAL returned), pkey_free() has well defined behavior which will be easier to maintain in one place.
For powerpc the return code is -1 rather than -EINVAL. This changes that behavior slightly but this is very unlikely to break any user space.
Lift the checks for pkey_free() to the core mm code and ensure consistency with returning -EINVAL.
Cc: ahaas@chromium.org Cc: clemensb@chromium.org Cc: gdeepti@chromium.org Cc: jkummerow@chromium.org Cc: manoskouk@chromium.org Cc: thibaudm@chromium.org Cc: Florian Weimer fweimer@redhat.com Cc: Andrew Morton akpm@linux-foundation.org Cc: linux-api@vger.kernel.org Cc: Sohil Mehta sohil.mehta@intel.com Cc: Dave Hansen dave.hansen@linux.intel.com Cc: Aneesh Kumar K.V aneesh.kumar@linux.ibm.com Signed-off-by: Ira Weiny ira.weiny@intel.com
Thanks to Sohil for suggesting I mention the powerpc return value in the commit message.
Also Sohil suggested changing mm_pkey_free() from int to void. This is added as a separate patch with his suggested by.
arch/powerpc/include/asm/pkeys.h | 6 ------ arch/x86/include/asm/pkeys.h | 3 --- mm/mprotect.c | 8 ++++++-- 3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 2c8351248793..e96aa91f817b 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -107,12 +107,6 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) static inline int mm_pkey_free(struct mm_struct *mm, int pkey) {
- if (!mmu_has_feature(MMU_FTR_PKEY))
return -1;- if (!mm_pkey_is_allocated(mm, pkey))
return -EINVAL;- __mm_pkey_free(mm, pkey);
return 0;
If it returns always 0, the return value is pointless and the function mm_pkey_free() should be changed to return void.
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h index 2e6c04d8a45b..da02737cc4d1 100644 --- a/arch/x86/include/asm/pkeys.h +++ b/arch/x86/include/asm/pkeys.h @@ -107,9 +107,6 @@ int mm_pkey_alloc(struct mm_struct *mm) static inline int mm_pkey_free(struct mm_struct *mm, int pkey) {
- if (!mm_pkey_is_allocated(mm, pkey))
return -EINVAL;- mm_set_pkey_free(mm, pkey);
return 0;
Same.
diff --git a/mm/mprotect.c b/mm/mprotect.c index 56d35de33725..41458e729c27 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -803,10 +803,14 @@ SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val) SYSCALL_DEFINE1(pkey_free, int, pkey) {
- int ret;
- int ret = -EINVAL;
Don't initialise 'ret'
- if (!arch_pkeys_enabled())
return ret;
Make it explicit, do 'return -EINVAL'
Once that is done, is there any point in having a fallback version of mm_pkey_free() which returns -EINVAL ?
mmap_write_lock(current->mm);
- ret = mm_pkey_free(current->mm, pkey);
- if (mm_pkey_is_allocated(current->mm, pkey))
ret = mm_pkey_free(current->mm, pkey);
Add:
else ret = -EINVAL;
mmap_write_unlock(current->mm); /*