Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

From: Yury Norov <ynorov@caviumnetworks.com>
To: "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-doc@vger.kernel.org>,
	Andrew Pinski <Andrew.Pinski@caviumnetworks.com>,
	<catalin.marinas@arm.com>, <heiko.carstens@de.ibm.com>,
	Hanjun Guo <guohanjun@huawei.com>, <joseph@codesourcery.com>,
	<linux-arch@vger.kernel.org>, <linux-s390@vger.kernel.org>,
	"jijun (D)" <jijun2@huawei.com>,
	<Prasun.Kapoor@caviumnetworks.com>, <schwab@suse.de>,
	<agraf@suse.de>, <pinskia@gmail.com>, <klimov.linux@gmail.com>,
	<broonie@kernel.org>, <Nathan_Lynch@mentor.com>,
	<linux-kernel@vger.kernel.org>,
	Andrew Pinski <apinski@cavium.com>, <schwidefsky@de.ibm.com>,
	<christoph.muellner@theobroma-systems.com>
Subject: Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it
Date: Thu, 12 May 2016 11:24:27 +0300	[thread overview]
Message-ID: <20160512082427.GA1587@yury-N73SV> (raw)
In-Reply-To: <5733FC71.8070101@huawei.com>

On Thu, May 12, 2016 at 11:45:53AM +0800, Zhangjian (Bamvor) wrote:

[...]

> >Hmm, that is indeed tricky. I think COMPAT_SYSCALL_WRAP4 rightfully
> >refuses the loff_t argument here, as the common case is that this is
> >not possible.
> It works if I apply the following patch, I defined the wrong `__TYPE_IS_xxx`
> yesterday. Should we merge this into ILP32 series or send the compat.h
> and syscalls.h individually? The current series of ILP32 is a little bit
> long and hard to review.
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index ba6ebe0..22a9565 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -747,7 +747,8 @@ asmlinkage long compat_sys_fanotify_mark(int, unsigned int, __u32, __u32,
>  #ifndef __SC_COMPAT_CAST
>  #define __SC_COMPAT_CAST(t, a) ({                                      \
>         BUILD_BUG_ON((sizeof(t) > 4) && !__TYPE_IS_L(t) &&              \
> -                    !__TYPE_IS_UL(t) && !__TYPE_IS_PTR(t));            \
> +                    !__TYPE_IS_UL(t) && !__TYPE_IS_PTR(t) &&           \
> +                    !__TYPE_IS_LOFFT(t));                              \

I think it's wrong, as loff_t is 64-bit in 32-bit userspace, and this
will clear meaningful data in top halve.

>         ((t) ((t)(-1) < 0 ? (s64)(s32)(a) : (u64)(u32)(a)));            \
>  })
>  #endif
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 6e57d9c..66eb85d 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -47,6 +47,7 @@
>  #define __TYPE_IS_L(t) (__same_type((t)0, 0L))
>  #define __TYPE_IS_UL(t)        (__same_type((t)0, 0UL))
>  #define __TYPE_IS_LL(t) (__same_type((t)0, 0LL) || __same_type((t)0, 0ULL))
> +#define __TYPE_IS_LOFFT(t)     (__same_type((t)0, (loff_t)0))
>  #define __SC_LONG(t, a) __typeof(__builtin_choose_expr(__TYPE_IS_LL(t), 0LL, 0L)) a
>  #define __SC_CAST(t, a)        (t) a
>  #define __SC_ARGS(t, a)        a
> diff --git a/kernel/compat_wrapper.c b/kernel/compat_wrapper.c
> index 98b68b8..28f02d0 100644
> --- a/kernel/compat_wrapper.c
> +++ b/kernel/compat_wrapper.c
> @@ -304,3 +304,7 @@ COMPAT_SYSCALL_WRAP3(getpeername, int, fd, struct sockaddr __user *, usockaddr,
>  COMPAT_SYSCALL_WRAP6(sendto, int, fd, void __user *, buff, size_t, len,
>                      unsigned int, flags, struct sockaddr __user *, addr,
>                      int, addr_len);
> +COMPAT_SYSCALL_WRAP4(pread64, unsigned int, fd, char __user *, buf,
> +                       size_t, count, loff_t, pos);
> +COMPAT_SYSCALL_WRAP4(pwrite64, unsigned int, fd, const char __user *, buf,
> +                        size_t, count, loff_t, pos);

For cases like this I think we should write wrappers by hands. In
unistd.h we can use __SC_WRAP, so they will work like wrappers
generated by COMPAT_SYSCALL_WRAPx() 