[COMPAT] Add compat_merge64 helper

[COMPAT] Add compat_merge64 helper

[Kyle McMartin]

To be used when endianness matters for argument ordering when reassembling
a 64-bit value out of two register halves.

Signed-off-by: Kyle McMartin <kyle@mcmartin.ca>
---
 include/linux/compat.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index 0e69d2c..07bcc3c 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -267,5 +267,14 @@ asmlinkage long compat_sys_signalfd(int ufd,
 asmlinkage long compat_sys_timerfd(int ufd, int clockid, int flags,
 				const struct compat_itimerspec __user *utmr);
 
+static inline u64 compat_merge64(u32 left, u32 right)
+{
+#if defined(__BIG_ENDIAN)
+	return ((u64)left << 32) | right;
+#else /* defined (__LITTLE_ENDIAN) */
+	return ((u64)right << 32) | left;
+#endif
+}
+	
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */
-- 
1.5.3.2

[PATCH] Generic compat_sys_fallocate

[Kyle McMartin]

Basically everyone is using the same sys32_fallocate. Delete a whole bunch of
archdep code and move the compat wrapper to fs/compat.c

Signed-off-by: Kyle McMartin <kyle@mcmartin.ca>
---
 arch/mips/kernel/linux32.c        |    7 -------
 arch/mips/kernel/scall64-o32.S    |    2 +-
 arch/powerpc/kernel/sys_ppc32.c   |    7 -------
 arch/sparc64/kernel/sys_sparc32.c |    6 ------
 arch/x86_64/ia32/ia32entry.S      |    2 +-
 arch/x86_64/ia32/sys_ia32.c       |    7 -------
 fs/compat.c                       |   10 ++++++++++
 include/linux/compat.h            |    2 ++
 8 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/arch/mips/kernel/linux32.c b/arch/mips/kernel/linux32.c
index 135d9a5..c37568d 100644
--- a/arch/mips/kernel/linux32.c
+++ b/arch/mips/kernel/linux32.c
@@ -566,13 +566,6 @@ asmlinkage long sys32_fadvise64_64(int fd, int __pad,
 			flags);
 }
 
-asmlinkage long sys32_fallocate(int fd, int mode, unsigned offset_a2,
-	unsigned offset_a3, unsigned len_a4, unsigned len_a5)
-{
-	return sys_fallocate(fd, mode, merge_64(offset_a2, offset_a3),
-	                     merge_64(len_a4, len_a5));
-}
-
 save_static_function(sys32_clone);
 static int noinline __used
 _sys32_clone(nabi_no_regargs struct pt_regs regs)
diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index b3ed731..bb724cc 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -525,5 +525,5 @@ sys_call_table:
 	PTR	compat_sys_signalfd
 	PTR	compat_sys_timerfd
 	PTR	sys_eventfd
-	PTR	sys_fallocate			/* 4320 */
+	PTR	compat_sys_fallocate		/* 4320 */
 	.size	sys_call_table,.-sys_call_table
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index bd85b5f..b42cbf1 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -773,13 +773,6 @@ asmlinkage int compat_sys_truncate64(const char __user * path, u32 reg4,
 	return sys_truncate(path, (high << 32) | low);
 }
 
-asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offhi, u32 offlo,
-				     u32 lenhi, u32 lenlo)
-{
-	return sys_fallocate(fd, mode, ((loff_t)offhi << 32) | offlo,
-			     ((loff_t)lenhi << 32) | lenlo);
-}
-
 asmlinkage int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long high,
 				 unsigned long low)
 {
diff --git a/arch/sparc64/kernel/sys_sparc32.c b/arch/sparc64/kernel/sys_sparc32.c
index e8dce90..5d60883 100644
--- a/arch/sparc64/kernel/sys_sparc32.c
+++ b/arch/sparc64/kernel/sys_sparc32.c
@@ -1028,9 +1028,3 @@ long compat_sync_file_range(int fd, unsigned long off_high, unsigned long off_lo
 				   flags);
 }
 
-asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offhi, u32 offlo,
-				     u32 lenhi, u32 lenlo)
-{
-	return sys_fallocate(fd, mode, ((loff_t)offhi << 32) | offlo,
-			     ((loff_t)lenhi << 32) | lenlo);
-}
diff --git a/arch/x86_64/ia32/ia32entry.S b/arch/x86_64/ia32/ia32entry.S
index 18b2318..28d8fff 100644
--- a/arch/x86_64/ia32/ia32entry.S
+++ b/arch/x86_64/ia32/ia32entry.S
@@ -732,5 +732,5 @@ ia32_sys_call_table:
 	.quad compat_sys_signalfd
 	.quad compat_sys_timerfd
 	.quad sys_eventfd
-	.quad sys32_fallocate
+	.quad compat_sys_fallocate
 ia32_syscall_end:
diff --git a/arch/x86_64/ia32/sys_ia32.c b/arch/x86_64/ia32/sys_ia32.c
index bee96d6..ca8deac 100644
--- a/arch/x86_64/ia32/sys_ia32.c
+++ b/arch/x86_64/ia32/sys_ia32.c
@@ -880,10 +880,3 @@ asmlinkage long sys32_fadvise64(int fd, unsigned offset_lo, unsigned offset_hi,
 				len, advice);
 }
 
-asmlinkage long sys32_fallocate(int fd, int mode, unsigned offset_lo,
-				unsigned offset_hi, unsigned len_lo,
-				unsigned len_hi)
-{
-	return sys_fallocate(fd, mode, ((u64)offset_hi << 32) | offset_lo,
-			     ((u64)len_hi << 32) | len_lo);
-}
diff --git a/fs/compat.c b/fs/compat.c
index 15078ce..9093915 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -2226,3 +2226,13 @@ asmlinkage long compat_sys_timerfd(int ufd, int clockid, int flags,
 }
 
 #endif /* CONFIG_TIMERFD */
+
+asmlinkage long compat_sys_fallocate(int fd, int mode,
+				     u32 offset_lo, u32 offset_hi,
+				     u32 len_lo, u32 len_hi)
+{
+	u64 offset = compat_merge64(offset_lo, offset_hi);
+	u64 len = compat_merge64(len_lo, len_hi);
+
+	return sys_fallocate(fd, mode, offset, len);
+}
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 07bcc3c..49401a9 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -266,6 +266,8 @@ asmlinkage long compat_sys_signalfd(int ufd,
                                 compat_size_t sigsetsize);
 asmlinkage long compat_sys_timerfd(int ufd, int clockid, int flags,
 				const struct compat_itimerspec __user *utmr);
+asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offset_lo,
+				u32 offset_hi, u32 len_lo, u32 len_hi);
 
 static inline u64 compat_merge64(u32 left, u32 right)
 {
-- 
1.5.3.2

Re: [PATCH] Generic compat_sys_fallocate

[Geert Uytterhoeven]

On Fri, 28 Sep 2007, Kyle McMartin wrote:
> --- a/fs/compat.c
> +++ b/fs/compat.c
> @@ -2226,3 +2226,13 @@ asmlinkage long compat_sys_timerfd(int ufd, int clockid, int flags,
>  }
>  
>  #endif /* CONFIG_TIMERFD */
> +
> +asmlinkage long compat_sys_fallocate(int fd, int mode,
> +				     u32 offset_lo, u32 offset_hi,
> +				     u32 len_lo, u32 len_hi)
> +{
> +	u64 offset = compat_merge64(offset_lo, offset_hi);
> +	u64 len = compat_merge64(len_lo, len_hi);
> +
> +	return sys_fallocate(fd, mode, offset, len);
> +}

To avoid confusion, you may want to rename the *_{lo,hi} parameters to
e.g. *_{l,r}, as they don't actually mean the low and high part anymore.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [COMPAT] Add compat_merge64 helper

[Arnd Bergmann]

On Saturday 29 September 2007, you wrote:
> +static inline u64 compat_merge64(u32 left, u32 right)
> +{
> +#if defined(__BIG_ENDIAN)
> +       return ((u64)left << 32) | right;
> +#else /* defined (__LITTLE_ENDIAN) */
> +       return ((u64)right << 32) | left;
> +#endif
> +}

Looks good, if we can guarantee that

1. Byte order matches the order in which 64 bit arguments are split
   in system call conventions on all platforms.
2. Every user of compat_merge64() includes asm/byteorder.h

Both should be easy to prove, but I'm not convinced until someone
actually does it.

	Arnd <><

Re: [COMPAT] Add compat_merge64 helper

[Kyle McMartin]

On Sat, Sep 29, 2007 at 01:38:23AM +0200, Arnd Bergmann wrote:
> 1. Byte order matches the order in which 64 bit arguments are split
>    in system call conventions on all platforms.

I checked powerpc, sparc, and mips, which are (besides parisc) the only
64-bit with 32-bit userspace big endian architectures that I could think
of offhand. A quick grep shows sh64 too... Paul?

> 2. Every user of compat_merge64() includes asm/byteorder.h
> 

*nod* Good point.

> Both should be easy to prove, but I'm not convinced until someone
> actually does it.
> 

Cheers,
	Kyle

Re: [COMPAT] Add compat_merge64 helper

[Arnd Bergmann]

On Saturday 29 September 2007, Kyle McMartin wrote:
> 
> On Sat, Sep 29, 2007 at 01:38:23AM +0200, Arnd Bergmann wrote:
> > 1. Byte order matches the order in which 64 bit arguments are split
> >    in system call conventions on all platforms.
> 
> I checked powerpc, sparc, and mips, which are (besides parisc) the only
> 64-bit with 32-bit userspace big endian architectures that I could think
> of offhand. A quick grep shows sh64 too... Paul?
> 

s390 is big-endian as well, and while it does have really weird C calling
conventions for 64 bit arguments in 32 bit mode, these do not affect the
system call ABI, so we should be fine here.

ia64 and x86_64 are obviously little-endian, and I double-checked that they
work with this logic.

	Arnd <><

Re: [COMPAT] Add compat_merge64 helper

[Kyle McMartin]

On Fri, Sep 28, 2007 at 08:01:31PM -0400, Kyle McMartin wrote:
> I checked powerpc, sparc, and mips, which are (besides parisc) the only
> 64-bit with 32-bit userspace big endian architectures that I could think
> of offhand. A quick grep shows sh64 too... Paul?
> 

Ah, no CONFIG_COMPAT on sh64 anyways.

Re: [COMPAT] Add compat_merge64 helper

[Paul Mundt]

On Fri, Sep 28, 2007 at 08:02:47PM -0400, Kyle McMartin wrote:
> On Fri, Sep 28, 2007 at 08:01:31PM -0400, Kyle McMartin wrote:
> > I checked powerpc, sparc, and mips, which are (besides parisc) the only
> > 64-bit with 32-bit userspace big endian architectures that I could think
> > of offhand. A quick grep shows sh64 too... Paul?
> > 
> 
> Ah, no CONFIG_COMPAT on sh64 anyways.

Yes, the kernel currently runs with a 32-bit ABI, so it's not an issue
there.

Re: [COMPAT] Add compat_merge64 helper

[Heiko Carstens]

On Fri, Sep 28, 2007 at 06:33:51PM -0400, Kyle McMartin wrote:
> To be used when endianness matters for argument ordering when reassembling
> a 64-bit value out of two register halves.
> 
> Signed-off-by: Kyle McMartin <kyle@mcmartin.ca>
>
> +static inline u64 compat_merge64(u32 left, u32 right)
> +{
> +#if defined(__BIG_ENDIAN)
> +	return ((u64)left << 32) | right;
> +#else /* defined (__LITTLE_ENDIAN) */

Could you change that to an #elif please and #error out if none is defined?
Should safe us from subtle bugs caused by missing includes.

Re: [COMPAT] Add compat_merge64 helper

[Ralf Baechle]

On Sat, Sep 29, 2007 at 11:48:52AM +0200, Heiko Carstens wrote:

> > +static inline u64 compat_merge64(u32 left, u32 right)
> > +{
> > +#if defined(__BIG_ENDIAN)
> > +	return ((u64)left << 32) | right;
> > +#else /* defined (__LITTLE_ENDIAN) */
> 
> Could you change that to an #elif please and #error out if none is defined?
> Should safe us from subtle bugs caused by missing includes.

This funny macro gets away without any extra headers or #ifdef messiness:

#define merge_64(r1,r2)							\
({									\
	union {								\
		int __words[2];						\
		long long	__dword;				\
	} __u = {							\
		.__words = { (r1), (r2) }				\
	};								\
									\
	__u.__dword;							\
})

Thanks to gcc doing bogus sign and zero extensions it compiles into
slightly larger code for MIPS but that may not be an issue on other
architectures.

   Ralf