linux: Change syscall return value to long int

  From: Xing Li <lixing@loongson.cn>

The kernel syscall return is long value.
The generic syscall interface return value
is int, which may lead to incorrect return value.

The following test is syscall with mmap executed on LoongArch,
only 32bits and sign extension value returned leading to mmap failure,
which should be with 47bits address returned.

Testcase:

 #include <sys/syscall.h>
 #include <sys/mman.h>
 #include <stdio.h>

void main()
{
        long int ret;
        ret = syscall(SYS_mmap, NULL, 0x801000, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
        printf("map address is %lx\n",ret);
}

Result:
[lixing@Sunhaiyong test]$ ./mmap
map address is fffffffff008c000
---
 sysdeps/unix/sysv/linux/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

The 11/29/2022 11:16, XingLi wrote:
> From: Xing Li <lixing@loongson.cn>
> 
> The kernel syscall return is long value.
> The generic syscall interface return value
> is int, which may lead to incorrect return value.

it's not clear what you mean here, the generic syscall
function returns long (according to unistd.h).

> 
> The following test is syscall with mmap executed on LoongArch,
> only 32bits and sign extension value returned leading to mmap failure,
> which should be with 47bits address returned.
> 
> Testcase:
> 
>  #include <sys/syscall.h>
>  #include <sys/mman.h>
>  #include <stdio.h>
> 
> void main()
> {
>         long int ret;
>         ret = syscall(SYS_mmap, NULL, 0x801000, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);

Note: there are many reasons why direct calls to syscall
may not work.

syscall is a variadic argument function that takes long
arguments, but you pass ints that may *not* be sign/zero
extended on the caller site so e.g. the top 32bits of
size and offset can be arbitrary on a 64bit system.
(you have to cast args to long to make the example valid).

and some systems use SYS_mmap2.

>         printf("map address is %lx\n",ret);
> }
> 
> Result:
> [lixing@Sunhaiyong test]$ ./mmap
> map address is fffffffff008c000
> ---
>  sysdeps/unix/sysv/linux/syscall.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/syscall.c b/sysdeps/unix/sysv/linux/syscall.c
> index 7303ba7188..8cb0b66b1c 100644
> --- a/sysdeps/unix/sysv/linux/syscall.c
> +++ b/sysdeps/unix/sysv/linux/syscall.c
> @@ -33,7 +33,7 @@ syscall (long int number, ...)
>    long int a5 = va_arg (args, long int);
>    va_end (args);
>  
> -  int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3, a4, a5);
> +  long int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3, a4, a5);
>    if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (r)))

this change looks reasonable to me.

>      {
>        __set_errno (-r);
> -- 
> 2.31.1
>
  

Xuerui: can you help to rewrite the commit message?  The code change
seems obvious but I'm not sure how to describe the rationale precisely
either.

On Tue, 2022-11-29 at 08:55 +0000, Szabolcs Nagy wrote:
> The 11/29/2022 11:16, XingLi wrote:
> > From: Xing Li <lixing@loongson.cn>
> > 
> > The kernel syscall return is long value.
> > The generic syscall interface return value
> > is int, which may lead to incorrect return value.
> 
> it's not clear what you mean here, the generic syscall
> function returns long (according to unistd.h).
> 
> > 
> > The following test is syscall with mmap executed on LoongArch,
> > only 32bits and sign extension value returned leading to mmap
> > failure,
> > which should be with 47bits address returned.
> > 
> > Testcase:
> > 
> >  #include <sys/syscall.h>
> >  #include <sys/mman.h>
> >  #include <stdio.h>
> > 
> > void main()
> > {
> >         long int ret;
> >         ret = syscall(SYS_mmap, NULL, 0x801000, PROT_READ |
> > PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
> 
> Note: there are many reasons why direct calls to syscall
> may not work.
> 
> syscall is a variadic argument function that takes long
> arguments, but you pass ints that may *not* be sign/zero
> extended on the caller site so e.g. the top 32bits of
> size and offset can be arbitrary on a 64bit system.
> (you have to cast args to long to make the example valid).
> 
> and some systems use SYS_mmap2.
> 
> >         printf("map address is %lx\n",ret);
> > }
> > 
> > Result:
> > [lixing@Sunhaiyong test]$ ./mmap
> > map address is fffffffff008c000
> > ---
> >  sysdeps/unix/sysv/linux/syscall.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/syscall.c
> > b/sysdeps/unix/sysv/linux/syscall.c
> > index 7303ba7188..8cb0b66b1c 100644
> > --- a/sysdeps/unix/sysv/linux/syscall.c
> > +++ b/sysdeps/unix/sysv/linux/syscall.c
> > @@ -33,7 +33,7 @@ syscall (long int number, ...)
> >    long int a5 = va_arg (args, long int);
> >    va_end (args);
> >  
> > -  int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3, a4,
> > a5);
> > +  long int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3,
> > a4, a5);
> >    if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (r)))
> 
> this change looks reasonable to me.
> 
> >      {
> >        __set_errno (-r);
> > -- 
> > 2.31.1
> >
  

The 11/29/2022 17:12, Xi Ruoyao wrote:
> Xuerui: can you help to rewrite the commit message?  The code change
> seems obvious but I'm not sure how to describe the rationale precisely
> either.

what about:

linux: Use long int for syscall return value

The linux syscall ABI returns long, so the generic syscall code for
linux should use long for the return value.

This fixes the truncation of the return value of the syscall function
when that does not fit into an int.
  

在 2022/11/29 下午5:37, Szabolcs Nagy 写道:
> The 11/29/2022 17:12, Xi Ruoyao wrote:
>> Xuerui: can you help to rewrite the commit message?  The code change
>> seems obvious but I'm not sure how to describe the rationale precisely
>> either.
> 
> what about:
> 
> linux: Use long int for syscall return value
> 
> The linux syscall ABI returns long, so the generic syscall code for
> linux should use long for the return value.
> 
> This fixes the truncation of the return value of the syscall function
> when that does not fit into an int.
> 
thanks