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

[Catalin Marinas <catalin.marinas@arm.com>]

(cc'ing Marcus for more insight on the tools side)

On Thu, Dec 17, 2015 at 12:14:20PM -0800, Andrew Pinski wrote:
> On Thu, Dec 17, 2015 at 12:10 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 17 December 2015 18:27:53 Catalin Marinas wrote:
> >> On Wed, Dec 16, 2015 at 12:42:38AM +0300, Yury Norov wrote:
> >
> >> > +#define compat_sys_lookup_dcookie      sys_lookup_dcookie
> >> > +#define compat_sys_pread64             sys_pread64
> >> > +#define compat_sys_pwrite64            sys_pwrite64
> >> > +#define compat_sys_readahead           sys_readahead
> >> > +#define compat_sys_shmat               sys_shmat
> >>
> >> I wonder whether we need wrappers (actually, not only for these but
> >> sys_read etc.). These functions take either a pointer or a size_t
> >> argument which are 32-bit with ILP32 but treated as 64-bit by an LP64
> >> kernel. Can we guarantee that user space zeros the top 32-bit of the
> >> arguments passed here?
> >
> > I'm pretty sure that is safe. I haven't read the calling conventions
> > specification for arm64 ilp32, but usually all function arguments are
> > passed as 64-bit registers with proper sign-extend or zero-extend.
> 
> Well (just like LP64 on AARCH64), when passing a 32bit value to a
> function, the upper 32bits are undefined.  I ran into this when I was
> debugging the GCC go library on ILP32 (though reproduced with pure C
> code) and the assembly functions inside glibc where pointers are
> passed with the upper 32bits as undefined.
> So we have an issue if called with syscall function or using pure
> assembly to create the syscall functions (which glibc does).

I think the ILP32 syscall ABI should follow the PCS convention where the
top 32-bit of a register is not guaranteed 0 when the size of the
argument is 32-bit. So take the read(2) syscall:

	ssize_t read(int fd, void *buf, size_t count);

>From the ILP32 code perspective, void * and size_t are both 32-bit. It
would call into the kernel leaving the top 32-bit as undefined (if we
follow the PCS). Normally, calling a function with the same size
arguments is not a problem since the compiler generates the callee code
accordingly. However, we route the syscall directly into the native
sys_read() where void * and size_t are 64-bit with the top 32-bit left
undefined.

We have three options here:

1. Always follow PCS convention across user/kernel call and add wrappers
   in the kernel (preferred)

2. Follow the PCS up to glibc and get glibc to zero the top part (not
   always safe with hand-written assembly, though we already do this for
   AArch32 where the PCS only specifies 4 arguments in registers, the
   rest go on the stack)

3. Follow the PCS up to glibc but always pass syscall arguments in W
   registers, like AArch32 compat support (the least preferred option,
   the only advantage is a single wrapper for all syscalls but it would
   be doing unnecessary zeroing even for syscalls where it isn't needed)

My preference, as stated above, is (1). You can write the wrappers in C
directly and let the compiler upgrade the types when calling the native
syscall. But any other option would be fine (take some inspiration from
other architectures). Unfortunately we don't have COMPAT_SYSCALL_DEFINE
for all functions that we need to wrap, it would have been easier (so we
need to add them but probably in the arch/arm64 code).

-- 
Catalin