| Message ID | 20200102145552.1853992-3-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On Thu, Jan 02, 2020 at 03:55:20PM +0100, Arnd Bergmann wrote: > In order to avoid needless #ifdef CONFIG_COMPAT checks, > move the compat_ptr() definition to linux/compat.h > where it can be seen by any file regardless of the > architecture. > > Only s390 needs a special definition, this can use the > self-#define trick we have elsewhere. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/arm64/include/asm/compat.h | 17 ----------------- > arch/mips/include/asm/compat.h | 18 ------------------ > arch/parisc/include/asm/compat.h | 17 ----------------- > arch/powerpc/include/asm/compat.h | 17 ----------------- > arch/powerpc/oprofile/backtrace.c | 2 +- > arch/s390/include/asm/compat.h | 6 +----- > arch/sparc/include/asm/compat.h | 17 ----------------- > arch/x86/include/asm/compat.h | 17 ----------------- > include/linux/compat.h | 18 ++++++++++++++++++ > 9 files changed, 20 insertions(+), 109 deletions(-) For arm64: Acked-by: Will Deacon <will@kernel.org> Will
On 2020-01-02 06:55, Arnd Bergmann wrote: > In order to avoid needless #ifdef CONFIG_COMPAT checks, > move the compat_ptr() definition to linux/compat.h > where it can be seen by any file regardless of the > architecture. > > Only s390 needs a special definition, this can use the > self-#define trick we have elsewhere. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/arm64/include/asm/compat.h | 17 ----------------- > arch/mips/include/asm/compat.h | 18 ------------------ > arch/parisc/include/asm/compat.h | 17 ----------------- > arch/powerpc/include/asm/compat.h | 17 ----------------- > arch/powerpc/oprofile/backtrace.c | 2 +- > arch/s390/include/asm/compat.h | 6 +----- > arch/sparc/include/asm/compat.h | 17 ----------------- > arch/x86/include/asm/compat.h | 17 ----------------- > include/linux/compat.h | 18 ++++++++++++++++++ > 9 files changed, 20 insertions(+), 109 deletions(-) > For x86: Reviewed-by: H. Peter Anvin <hpa@zytor.com> It still suffers from the zero-one-infinity rule failure of the compat architecture as a whole, but that is a very different problem. In this case "compat" is obviously meaning "a 32-on-64 ABI" and simply centralizes a common API, which is a Good Thing[TM]. -hpa
Arnd Bergmann <arnd@arndb.de> writes: > In order to avoid needless #ifdef CONFIG_COMPAT checks, > move the compat_ptr() definition to linux/compat.h > where it can be seen by any file regardless of the > architecture. > > Only s390 needs a special definition, this can use the > self-#define trick we have elsewhere. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/arm64/include/asm/compat.h | 17 ----------------- > arch/mips/include/asm/compat.h | 18 ------------------ > arch/parisc/include/asm/compat.h | 17 ----------------- > arch/powerpc/include/asm/compat.h | 17 ----------------- > arch/powerpc/oprofile/backtrace.c | 2 +- LGTM. Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) One minor comment: > diff --git a/include/linux/compat.h b/include/linux/compat.h > index 68f79d855c3d..11083d84eb23 100644 > --- a/include/linux/compat.h > +++ b/include/linux/compat.h > @@ -958,4 +958,22 @@ static inline bool in_compat_syscall(void) { return false; } > > #endif /* CONFIG_COMPAT */ > > +/* > + * A pointer passed in from user mode. This should not > + * be used for syscall parameters, just declare them > + * as pointers because the syscall entry code will have > + * appropriately converted them already. > + */ > +#ifndef compat_ptr > +static inline void __user *compat_ptr(compat_uptr_t uptr) > +{ > + return (void __user *)(unsigned long)uptr; > +} > +#endif > + > +static inline compat_uptr_t ptr_to_compat(void __user *uptr) > +{ > + return (u32)(unsigned long)uptr; > +} Is there a reason we cast to u32 directly instead of using compat_uptr_t? cheers
On Tue, Jan 7, 2020 at 3:05 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > Arnd Bergmann <arnd@arndb.de> writes: > > + > > +static inline compat_uptr_t ptr_to_compat(void __user *uptr) > > +{ > > + return (u32)(unsigned long)uptr; > > +} > > Is there a reason we cast to u32 directly instead of using compat_uptr_t? Probably Al found this to be more explicit at the time when he introduced it on all the architectures in 2005. I just moved it here and kept the definition. Arnd
<linuxppc-dev@lists.ozlabs.org>,oprofile-list@lists.sf.net,linux-s390 <linux-s390@vger.kernel.org>,sparclinux <sparclinux@vger.kernel.org> From: hpa@zytor.com Message-ID: <41625F06-D755-4C82-86DF-A9415FEEE13D@zytor.com> On January 7, 2020 12:08:31 AM PST, Arnd Bergmann <arnd@arndb.de> wrote: >On Tue, Jan 7, 2020 at 3:05 AM Michael Ellerman <mpe@ellerman.id.au> >wrote: >> Arnd Bergmann <arnd@arndb.de> writes: >> > + >> > +static inline compat_uptr_t ptr_to_compat(void __user *uptr) >> > +{ >> > + return (u32)(unsigned long)uptr; >> > +} >> >> Is there a reason we cast to u32 directly instead of using >compat_uptr_t? > >Probably Al found this to be more explicit at the time when he >introduced >it on all the architectures in 2005. I just moved it here and kept the >definition. > > Arnd Did compat_uptr_t exist back then?
On Tue, Jan 7, 2020 at 9:20 AM H. Peter Anvin <hpa@zytor.com> wrote: > > <linuxppc-dev@lists.ozlabs.org>,oprofile-list@lists.sf.net,linux-s390 <linux-s390@vger.kernel.org>,sparclinux <sparclinux@vger.kernel.org> > From: hpa@zytor.com > Message-ID: <41625F06-D755-4C82-86DF-A9415FEEE13D@zytor.com> > > On January 7, 2020 12:08:31 AM PST, Arnd Bergmann <arnd@arndb.de> wrote: > >On Tue, Jan 7, 2020 at 3:05 AM Michael Ellerman <mpe@ellerman.id.au> > >wrote: > >> Arnd Bergmann <arnd@arndb.de> writes: > >> > + > >> > +static inline compat_uptr_t ptr_to_compat(void __user *uptr) > >> > +{ > >> > + return (u32)(unsigned long)uptr; > >> > +} > >> > >> Is there a reason we cast to u32 directly instead of using > >compat_uptr_t? > > > >Probably Al found this to be more explicit at the time when he > >introduced > >it on all the architectures in 2005. I just moved it here and kept the > >definition. > > > > Arnd > > Did compat_uptr_t exist back then? Yes, compat_uptr_t (and the earlier compat_ptr_t) goes back to at least 2003 with the introduction of compat_ptr(). These are some related commits from bitkeeper: commit 29d04cca305a93cfa90afe1c6440ce5421837213 Author: Stephen Rothwell <sfr@canb.auug.org.au> Date: Sun Mar 23 04:59:16 2003 -0800 [PATCH] compat_uptr_t and compat_ptr This creates compat_uptr_t (to represent a user mode pointer passed to the kernel other than as a syscall parameter) and compat_ptr() to convert it to a kernel pointer. This fixes a couple of bugs in s390x (where the conversion of pointers actually does something). diff --git a/include/asm-s390x/compat.h b/include/asm-s390x/compat.h index 55e23c9b36fe..d1e948ae4bff 100644 --- a/include/asm-s390x/compat.h +++ b/include/asm-s390x/compat.h @@ -101,4 +101,17 @@ typedef u32 compat_sigset_word; #define COMPAT_OFF_T_MAX 0x7fffffff #define COMPAT_LOFF_T_MAX 0x7fffffffffffffffL +/* + * A pointer passed in from user mode. This should not + * be used for syscall parameters, just declare them + * as pointers because the syscall entry code will have + * appropriately comverted them already. + */ +typedef u32 compat_uptr_t; + +static inline void *compat_ptr(compat_ptr_t uptr) +{ + return (void *)(uptr & 0x7fffffffUL); +} + #endif /* _ASM_S390X_COMPAT_H */ commit 9d814250b08868cb39cc5a4f43f5cbf5e517f5f4 Author: Alexander Viro <viro@parcelfarce.linux.theplanet.co.uk> Date: Tue Feb 1 21:21:39 2005 -0800 [PATCH] sparc64 compat annotations same story as for amd64 - ptr_to_compat() + normal __user annotations Signed-off-by: Al Viro <viro@parcelfarce.linux.theplanet.co.uk> Signed-off-by: Linus Torvalds <torvalds@osdl.org> diff --git a/include/asm-sparc64/compat.h b/include/asm-sparc64/compat.h index 874b136ded90..e26d7d85c55a 100644 --- a/include/asm-sparc64/compat.h +++ b/include/asm-sparc64/compat.h @@ -121,6 +121,11 @@ static inline void __user *compat_ptr(compat_uptr_t uptr) return (void __user *)(unsigned long)uptr; } +static inline compat_uptr_t ptr_to_compat(void __user *uptr) +{ + return (u32)(unsigned long)uptr; +} + static __inline__ void __user *compat_alloc_user_space(long len) { struct pt_regs *regs = current_thread_info()->kregs;
On Thu, Jan 02, 2020 at 03:55:20PM +0100, Arnd Bergmann wrote: > In order to avoid needless #ifdef CONFIG_COMPAT checks, > move the compat_ptr() definition to linux/compat.h > where it can be seen by any file regardless of the > architecture. > > Only s390 needs a special definition, this can use the > self-#define trick we have elsewhere. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/arm64/include/asm/compat.h | 17 ----------------- > arch/mips/include/asm/compat.h | 18 ------------------ > arch/parisc/include/asm/compat.h | 17 ----------------- > arch/powerpc/include/asm/compat.h | 17 ----------------- > arch/powerpc/oprofile/backtrace.c | 2 +- > arch/s390/include/asm/compat.h | 6 +----- > arch/sparc/include/asm/compat.h | 17 ----------------- > arch/x86/include/asm/compat.h | 17 ----------------- > include/linux/compat.h | 18 ++++++++++++++++++ > 9 files changed, 20 insertions(+), 109 deletions(-) For s390: Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index 7b4172ce497c..935d2aa231bf 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -114,23 +114,6 @@ typedef u32 compat_sigset_word; #define COMPAT_OFF_T_MAX 0x7fffffff -/* - * A pointer passed in from user mode. This should not - * be used for syscall parameters, just declare them - * as pointers because the syscall entry code will have - * appropriately converted them already. - */ - -static inline void __user *compat_ptr(compat_uptr_t uptr) -{ - return (void __user *)(unsigned long)uptr; -} - -static inline compat_uptr_t ptr_to_compat(void __user *uptr) -{ - return (u32)(unsigned long)uptr; -} - #define compat_user_stack_pointer() (user_stack_pointer(task_pt_regs(current))) #define COMPAT_MINSIGSTKSZ 2048 diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h index c99166eadbde..255afcdd79c9 100644 --- a/arch/mips/include/asm/compat.h +++ b/arch/mips/include/asm/compat.h @@ -100,24 +100,6 @@ typedef u32 compat_sigset_word; #define COMPAT_OFF_T_MAX 0x7fffffff -/* - * A pointer passed in from user mode. This should not - * be used for syscall parameters, just declare them - * as pointers because the syscall entry code will have - * appropriately converted them already. - */ - -static inline void __user *compat_ptr(compat_uptr_t uptr) -{ - /* cast to a __user pointer via "unsigned long" makes sparse happy */ - return (void __user *)(unsigned long)(long)uptr; -} - -static inline compat_uptr_t ptr_to_compat(void __user *uptr) -{ - return (u32)(unsigned long)uptr; -} - static inline void __user *arch_compat_alloc_user_space(long len) { struct pt_regs *regs = (struct pt_regs *) diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h index e03e3c849f40..2f4f66a3bac0 100644 --- a/arch/parisc/include/asm/compat.h +++ b/arch/parisc/include/asm/compat.h @@ -173,23 +173,6 @@ struct compat_shmid64_ds { #define COMPAT_ELF_NGREG 80 typedef compat_ulong_t compat_elf_gregset_t[COMPAT_ELF_NGREG]; -/* - * A pointer passed in from user mode. This should not - * be used for syscall parameters, just declare them - * as pointers because the syscall entry code will have - * appropriately converted them already. - */ - -static inline void __user *compat_ptr(compat_uptr_t uptr) -{ - return (void __user *)(unsigned long)uptr; -} - -static inline compat_uptr_t ptr_to_compat(void __user *uptr) -{ - return (u32)(unsigned long)uptr; -} - static __inline__ void __user *arch_compat_alloc_user_space(long len) { struct pt_regs *regs = ¤t->thread.regs; diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h index 74d0db511099..3e3cdfaa76c6 100644 --- a/arch/powerpc/include/asm/compat.h +++ b/arch/powerpc/include/asm/compat.h @@ -96,23 +96,6 @@ typedef u32 compat_sigset_word; #define COMPAT_OFF_T_MAX 0x7fffffff -/* - * A pointer passed in from user mode. This should not - * be used for syscall parameters, just declare them - * as pointers because the syscall entry code will have - * appropriately converted them already. - */ - -static inline void __user *compat_ptr(compat_uptr_t uptr) -{ - return (void __user *)(unsigned long)uptr; -} - -static inline compat_uptr_t ptr_to_compat(void __user *uptr) -{ - return (u32)(unsigned long)uptr; -} - static inline void __user *arch_compat_alloc_user_space(long len) { struct pt_regs *regs = current->thread.regs; diff --git a/arch/powerpc/oprofile/backtrace.c b/arch/powerpc/oprofile/backtrace.c index 43245f4a9bcb..6ffcb80cf844 100644 --- a/arch/powerpc/oprofile/backtrace.c +++ b/arch/powerpc/oprofile/backtrace.c @@ -9,7 +9,7 @@ #include <linux/sched.h> #include <asm/processor.h> #include <linux/uaccess.h> -#include <asm/compat.h> +#include <linux/compat.h> #include <asm/oprofile_impl.h> #define STACK_SP(STACK) *(STACK) diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h index 63b46e30b2c3..9547cd5d6cdc 100644 --- a/arch/s390/include/asm/compat.h +++ b/arch/s390/include/asm/compat.h @@ -177,11 +177,7 @@ static inline void __user *compat_ptr(compat_uptr_t uptr) { return (void __user *)(unsigned long)(uptr & 0x7fffffffUL); } - -static inline compat_uptr_t ptr_to_compat(void __user *uptr) -{ - return (u32)(unsigned long)uptr; -} +#define compat_ptr(uptr) compat_ptr(uptr) #ifdef CONFIG_COMPAT diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h index 30b1763580b1..40a267b3bd52 100644 --- a/arch/sparc/include/asm/compat.h +++ b/arch/sparc/include/asm/compat.h @@ -125,23 +125,6 @@ typedef u32 compat_sigset_word; #define COMPAT_OFF_T_MAX 0x7fffffff -/* - * A pointer passed in from user mode. This should not - * be used for syscall parameters, just declare them - * as pointers because the syscall entry code will have - * appropriately converted them already. - */ - -static inline void __user *compat_ptr(compat_uptr_t uptr) -{ - return (void __user *)(unsigned long)uptr; -} - -static inline compat_uptr_t ptr_to_compat(void __user *uptr) -{ - return (u32)(unsigned long)uptr; -} - #ifdef CONFIG_COMPAT static inline void __user *arch_compat_alloc_user_space(long len) { diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h index 22c4dfe65992..52e9f3480f69 100644 --- a/arch/x86/include/asm/compat.h +++ b/arch/x86/include/asm/compat.h @@ -177,23 +177,6 @@ typedef struct user_regs_struct compat_elf_gregset_t; (!!(task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT)) #endif -/* - * A pointer passed in from user mode. This should not - * be used for syscall parameters, just declare them - * as pointers because the syscall entry code will have - * appropriately converted them already. - */ - -static inline void __user *compat_ptr(compat_uptr_t uptr) -{ - return (void __user *)(unsigned long)uptr; -} - -static inline compat_uptr_t ptr_to_compat(void __user *uptr) -{ - return (u32)(unsigned long)uptr; -} - static inline void __user *arch_compat_alloc_user_space(long len) { compat_uptr_t sp; diff --git a/include/linux/compat.h b/include/linux/compat.h index 68f79d855c3d..11083d84eb23 100644 --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -958,4 +958,22 @@ static inline bool in_compat_syscall(void) { return false; } #endif /* CONFIG_COMPAT */ +/* + * A pointer passed in from user mode. This should not + * be used for syscall parameters, just declare them + * as pointers because the syscall entry code will have + * appropriately converted them already. + */ +#ifndef compat_ptr +static inline void __user *compat_ptr(compat_uptr_t uptr) +{ + return (void __user *)(unsigned long)uptr; +} +#endif + +static inline compat_uptr_t ptr_to_compat(void __user *uptr) +{ + return (u32)(unsigned long)uptr; +} + #endif /* _LINUX_COMPAT_H */
In order to avoid needless #ifdef CONFIG_COMPAT checks, move the compat_ptr() definition to linux/compat.h where it can be seen by any file regardless of the architecture. Only s390 needs a special definition, this can use the self-#define trick we have elsewhere. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/arm64/include/asm/compat.h | 17 ----------------- arch/mips/include/asm/compat.h | 18 ------------------ arch/parisc/include/asm/compat.h | 17 ----------------- arch/powerpc/include/asm/compat.h | 17 ----------------- arch/powerpc/oprofile/backtrace.c | 2 +- arch/s390/include/asm/compat.h | 6 +----- arch/sparc/include/asm/compat.h | 17 ----------------- arch/x86/include/asm/compat.h | 17 ----------------- include/linux/compat.h | 18 ++++++++++++++++++ 9 files changed, 20 insertions(+), 109 deletions(-)