Re: [PATCH v3 21/31] arm64: 32-bit (compat) applications support

[Catalin Marinas]

On Thu, Sep 13, 2012 at 12:03:27PM +0100, Arnd Bergmann wrote:
> On Thursday 13 September 2012, Catalin Marinas wrote:
> > +#ifdef __ARCH_WANT_COMPAT_SYS_SENDFILE
> > +asmlinkage int compat_sys_sendfile(int out_fd, int in_fd,
> > +                                  compat_off_t __user *offset, s32 count)
> > +{
> > +       mm_segment_t old_fs = get_fs();
> > +       int ret;
> > +       off_t of;
> > +
> > +       if (offset && get_user(of, offset))
> > +               return -EFAULT;
> > +
> > +       set_fs(KERNEL_DS);
> > +       ret = sys_sendfile(out_fd, in_fd,
> > +                          offset ? (off_t __user *)&of : NULL, count);
> > +       set_fs(old_fs);
> > +
> > +       if (offset && put_user(of, offset))
> > +               return -EFAULT;
> > +       return ret;
> > +}
> > +#endif /* __ARCH_WANT_COMPAT_SYS_SENDFILE */
> 
> Looking at this code in detail now, I think it's better to move the functions to
> fs/read_write.c and get rid of the get_fs/set_fs hack, like
> 
> asmlinkage int compat_sys_sendfile(int out_fd, int in_fd,
>                                    compat_off_t __user *offset, s32 count)
...

It makes sense (copied below, just for compat_sys_sendfile until my
comments below are clarified). One minor improvement, I think we should
use compat_size_t for the count, it is a u32 in all cases.

> This implementation is smaller and more efficient than the common one.
> 
> Same for compat_sys_sendfile64, although I don't think there is ever
> a case where loff_t is defined differently from compat_loff_t, so
> you can probably just use the native sys_sendfile64 for the compat
> case.

That's what we do on AArch64, though powerpc and sparc define their own.
The "count" argument would be different between compat and non-compat
versions and I'm not sure what assumptions are made on the syscall entry
path on these architectures (on AArch64 we ensure that the top 32-bit
part of an X register is always 0 for 32-bit syscalls).

Another difference is that the "count" argument for
compat_sys_sendfile64 is s32 on powerpc and u32 on sparc. I can't tell
whether it would make a difference in practice but if we use
compat_size_t for the generic version the powerpc wouldn't get the sign
extension.

Powerpc has some comment about a need to treat in_fd/out_fd arguments as
signed ints but I don't fully understand it (well, maybe powerpc needs
the sign to be fully extended to 64-bit even for int).

----------8<-----------------------