| Message ID | 20220610233533.3649584-6-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | User pkey minor bug fixes | expand |
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); > > /*
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; 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; 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; + + if (!arch_pkeys_enabled()) + return ret; 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); mmap_write_unlock(current->mm); /*