Re: sparc_pipe(2)

Re: sparc_pipe(2)

[David Miller]

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 20 Mar 2018 00:12:24 +0000

> Am I missing something here, or is it simply that the thing predates
> current_pt_regs()?

It probably just predates current_pt_regs(), yes.

Re: sparc_pipe(2)

[Al Viro]

On Mon, Mar 19, 2018 at 09:11:25PM -0400, David Miller wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Tue, 20 Mar 2018 00:12:24 +0000
> 
> > Am I missing something here, or is it simply that the thing predates
> > current_pt_regs()?
> 
> It probably just predates current_pt_regs(), yes.

OK...  Another fun question in the same area:
asmlinkage long sys32_ftruncate64(unsigned int fd, unsigned long high, unsigned long low)
{
        if ((int)high < 0)
                return -EINVAL;
        else
                return sys_ftruncate(fd, (high << 32) | low);
}

Is there any reason we want to check high right there?  After all,
sys_ftruncate() will produce exactly that on MSB of its second
argument set...  The same goes for sys32_truncate() - sys_truncate()
will yield -EINVAL on negative loff_t.

Re: sparc_pipe(2)

[David Miller]

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 20 Mar 2018 03:42:00 +0000

> On Mon, Mar 19, 2018 at 09:11:25PM -0400, David Miller wrote:
>> From: Al Viro <viro@ZenIV.linux.org.uk>
>> Date: Tue, 20 Mar 2018 00:12:24 +0000
>> 
>> > Am I missing something here, or is it simply that the thing predates
>> > current_pt_regs()?
>> 
>> It probably just predates current_pt_regs(), yes.
> 
> OK...  Another fun question in the same area:
> asmlinkage long sys32_ftruncate64(unsigned int fd, unsigned long high, unsigned long low)
> {
>         if ((int)high < 0)
>                 return -EINVAL;
>         else
>                 return sys_ftruncate(fd, (high << 32) | low);
> }
> 
> Is there any reason we want to check high right there?  After all,
> sys_ftruncate() will produce exactly that on MSB of its second
> argument set...  The same goes for sys32_truncate() - sys_truncate()
> will yield -EINVAL on negative loff_t.

Indeed, both checks look like they are extraneous.

Re: sparc_pipe(2)

[Al Viro]

On Tue, Mar 20, 2018 at 10:13:02AM -0400, David Miller wrote:

> > Is there any reason we want to check high right there?  After all,
> > sys_ftruncate() will produce exactly that on MSB of its second
> > argument set...  The same goes for sys32_truncate() - sys_truncate()
> > will yield -EINVAL on negative loff_t.
> 
> Indeed, both checks look like they are extraneous.

While looking through the syscall wrappers there:
sys_sigreturn:  
        call    do_sigreturn
         add    %sp, STACKFRAME_SZ, %o0

        ld      [%curptr + TI_FLAGS], %l5
        andcc   %l5, _TIF_SYSCALL_TRACE, %g0
        be      1f
         nop

        call    syscall_trace
         mov    1, %o1
which seems to rely upon do_sigreturn() leaving its %i0 alone, while
sys_rt_sigreturn:
        call    do_rt_sigreturn
         add    %sp, STACKFRAME_SZ, %o0

        ld      [%curptr + TI_FLAGS], %l5
        andcc   %l5, _TIF_SYSCALL_TRACE, %g0
        be      1f
         nop

        add     %sp, STACKFRAME_SZ, %o0
        call    syscall_trace
         mov    1, %o1
doesn't count upon the same for do_rt_sigreturn().  I'm not saying that
any of that is in any way critical (sparc32 kernel, for fsck sake...),
just curious about the difference here.  IIRC, there's nothing to
prevent void foo(void *p) stomping on its %i0; it's unlikely to happen
in either case, but why the difference in callers?

Anyway, I've put together some cleanups ({COMPAT_,}SYSCALL_DEFINE
conversions, getting rid of SIGN... wrappers) in
git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git misc.sparc
Do you see any obviour problems with the stuff in there?  It's not
urgent - the real fun with compat wrappers will be on mips and s390,
anyway; sparc is fairly benign in that respect...

Re: sparc_pipe(2)

[David Miller]

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 20 Mar 2018 17:02:26 +0000

> While looking through the syscall wrappers there:
> sys_sigreturn:  
>         call    do_sigreturn
>          add    %sp, STACKFRAME_SZ, %o0
> 
>         ld      [%curptr + TI_FLAGS], %l5
>         andcc   %l5, _TIF_SYSCALL_TRACE, %g0
>         be      1f
>          nop
> 
>         call    syscall_trace
>          mov    1, %o1
> which seems to rely upon do_sigreturn() leaving its %i0 alone, while
> sys_rt_sigreturn:
>         call    do_rt_sigreturn
>          add    %sp, STACKFRAME_SZ, %o0
> 
>         ld      [%curptr + TI_FLAGS], %l5
>         andcc   %l5, _TIF_SYSCALL_TRACE, %g0
>         be      1f
>          nop
> 
>         add     %sp, STACKFRAME_SZ, %o0
>         call    syscall_trace
>          mov    1, %o1
> doesn't count upon the same for do_rt_sigreturn().  I'm not saying that
> any of that is in any way critical (sparc32 kernel, for fsck sake...),
> just curious about the difference here.  IIRC, there's nothing to
> prevent void foo(void *p) stomping on its %i0; it's unlikely to happen
> in either case, but why the difference in callers?

Good catch, I think %o0 has to be reloaded in the do_sigreturn case.

> Anyway, I've put together some cleanups ({COMPAT_,}SYSCALL_DEFINE
> conversions, getting rid of SIGN... wrappers) in
> git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git misc.sparc
> Do you see any obviour problems with the stuff in there?  It's not
> urgent - the real fun with compat wrappers will be on mips and s390,
> anyway; sparc is fairly benign in that respect...

Nothing in there seems objectionable to me.

Thanks Al!

Re: sparc_pipe(2)

[Al Viro]

On Tue, Mar 20, 2018 at 02:22:48PM -0400, David Miller wrote:

> > Anyway, I've put together some cleanups ({COMPAT_,}SYSCALL_DEFINE
> > conversions, getting rid of SIGN... wrappers) in
> > git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git misc.sparc
> > Do you see any obviour problems with the stuff in there?  It's not
> > urgent - the real fun with compat wrappers will be on mips and s390,
> > anyway; sparc is fairly benign in that respect...
> 
> Nothing in there seems objectionable to me.

More oddities: in commit b340b81a6554facb1b2fbe68691e254d53315d20
Author: David S. Miller <davem@nuts.ninka.net>
Date:   Sun Nov 3 09:24:46 2002 -0800

    [SPARC]: Add sys_remap_file_pages syscalls.

we have
int sparc_remap_file_pages(unsigned long start, unsigned long size,
                          unsigned long prot, unsigned long pgoff,
                          unsigned long flags)
{
       /* This works on an existing mmap so we don't need to validate
        * the range as that was done at the original mmap call.
        */
       return sys_remap_file_pages(start, size, prot,
                                   (pgoff >> (PAGE_SHIFT - 12)), flags);
}

put into native 32bit syscall table and plain sys_remap_file_pages() into
64bit ones - both native and compat.  AFAICS, that would have
remap_file_pages() in 32bit binary operate in units of 4Kb on 32bit
host and PAGE_SIZE - on 64bit one.

Confused...

Other interesting differences:
	* no getpeername() or getsockname() in compat table; AFAICS,
for both the native syscall should work...
	* #254 is ni_syscall() on native an nis_syscall() on compat,
#267 is the other way round.  Huh?  What are the rules for ni vs. nis,
anyway?  I understand that the former is quiet and the latter reports
attempts to issue the syscall in question, but how do we choose which
one to use for given unimplemented syscall?

Re: sparc_pipe(2)

[David Miller]

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 20 Mar 2018 23:02:12 +0000

>     [SPARC]: Add sys_remap_file_pages syscalls.
> 
> we have
> int sparc_remap_file_pages(unsigned long start, unsigned long size,
>                           unsigned long prot, unsigned long pgoff,
>                           unsigned long flags)
> {
>        /* This works on an existing mmap so we don't need to validate
>         * the range as that was done at the original mmap call.
>         */
>        return sys_remap_file_pages(start, size, prot,
>                                    (pgoff >> (PAGE_SHIFT - 12)), flags);
> }
> 
> put into native 32bit syscall table and plain sys_remap_file_pages() into
> 64bit ones - both native and compat.  AFAICS, that would have
> remap_file_pages() in 32bit binary operate in units of 4Kb on 32bit
> host and PAGE_SIZE - on 64bit one.

That's bogus, 32-bit binary should use system page size, rather than
a fixed 4K value.

> Other interesting differences:
> 	* no getpeername() or getsockname() in compat table; AFAICS,
> for both the native syscall should work...

The 32-bit userspace never used these and instead went through
socketcall(), so I simply never added them to the syscall table.

> 	* #254 is ni_syscall() on native an nis_syscall() on compat,
> #267 is the other way round.  Huh?  What are the rules for ni vs. nis,
> anyway?  I understand that the former is quiet and the latter reports
> attempts to issue the syscall in question, but how do we choose which
> one to use for given unimplemented syscall?

I think they should all be quiet, and the warning is of limited to no
value these days.