[PATCH] nptl: Fix SYSCALL_CANCEL for return values larger than INT_MAX (BZ 33245)

[Adhemerval Zanella Netto]

On 02/08/25 16:07, Sam James wrote:
> Sam James <sam@gentoo.org> writes:
> 
>> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>
>>> The SYSCALL_CANCEL calls __syscall_cancel, which in turn
>>> calls __internal_syscall_cancel with an 'int' return instead of the
>>> expected 'long int'.  This causes issues with syscalls that return
>>> values larger than INT_MAX, such as copy_file_range [1].
>>>
>>> Checked on x86_64-linux-gnu.
>>>
>>> [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=79139
>>
>> The patch is OK:
>>
>> Reviewed-by: Sam James <sam@gentoo.org>
>>
>>> ---
>>>  nptl/cancellation.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> but I have a question about some other instances, though I didn't look
>> afresh yet today, just from last night.
>>
>> Do we need something like the attached? I think the readlink one doesn't
>> actually matter because of path sizes.
> 
> OK, looked more closely..
> 
>>
>> diff --git a/sysdeps/unix/sysv/linux/dl-origin.c b/sysdeps/unix/sysv/linux/dl-origin.c
>> index 3c52ba51a6..b7fec7cc3f 100644
>> --- a/sysdeps/unix/sysv/linux/dl-origin.c
>> +++ b/sysdeps/unix/sysv/linux/dl-origin.c
>> @@ -32,7 +32,7 @@ _dl_get_origin (void)
>>  {
>>    char linkval[PATH_MAX];
>>    char *result;
>> -  int len;
>> +  long len;
>>  
>>    len = INTERNAL_SYSCALL_CALL (readlinkat, AT_FDCWD, "/proc/self/exe",
>>  			       linkval, sizeof (linkval));
>> @@ -84,7 +84,7 @@ _dl_canonicalize (int fd)
>>    struct fd_to_filename fdfilename;
>>    char canonical[PATH_MAX];
>>    char *path = __fd_to_filename (fd, &fdfilename);
>> -  int size = INTERNAL_SYSCALL_CALL (readlinkat, AT_FDCWD, path,
>> +  long size = INTERNAL_SYSCALL_CALL (readlinkat, AT_FDCWD, path,
>>                                      canonical, PATH_MAX - 1);
> 
> Again, doesn't really matter in practice AFAICT.
> 
>>  
>>    /* Check if the path was truncated.  */
>> diff --git a/sysdeps/unix/sysv/linux/lseek.c b/sysdeps/unix/sysv/linux/lseek.c
>> index ff83735d09..9a1383f782 100644
>> --- a/sysdeps/unix/sysv/linux/lseek.c
>> +++ b/sysdeps/unix/sysv/linux/lseek.c
>> @@ -47,7 +47,7 @@ __lseek (int fd, off_t offset, int whence)
>>  
>>  # ifdef __NR__llseek
>>    loff_t res;
>> -  int rc = INLINE_SYSCALL_CALL (_llseek, fd,
>> +  long int rc = INLINE_SYSCALL_CALL (_llseek, fd,
>>  				(long) (((uint64_t) (offset)) >> 32),
>>  				(long) offset, &res, whence);
>>    return rc ?: lseek_overflow (res);
> 
> *llseek* uses a parameter for its return value, so it should be fine. If
> the arch lacks llseek, the other path is fine.
> 
>> diff --git a/sysdeps/unix/sysv/linux/lseek64.c b/sysdeps/unix/sysv/linux/lseek64.c
>> index 49f4136844..b5624388cc 100644
>> --- a/sysdeps/unix/sysv/linux/lseek64.c
>> +++ b/sysdeps/unix/sysv/linux/lseek64.c
>> @@ -32,7 +32,7 @@ __lseek64 (int fd, off64_t offset, int whence)
>>  
>>  #ifdef __NR__llseek
>>    loff_t res;
>> -  int rc = INLINE_SYSCALL_CALL (_llseek, fd,
>> +  long int rc = INLINE_SYSCALL_CALL (_llseek, fd,
>>  				(long) (((uint64_t) (offset)) >> 32),
>>  				(long) offset, &res, whence);
> 
> Ditto llseek/lseek above.

The rc is just used to signal an error, where the kABI will set -errno in
this case (which is handled by INLINE_SYSCALL_CALL). On success the
'res' will be returned, which is 'long long'.

> 
>>    return rc ?: res;
>> diff --git a/sysdeps/unix/sysv/linux/sendfile.c b/sysdeps/unix/sysv/linux/sendfile.c
>> index 990a26a3ea..1f04ce5aab 100644
>> --- a/sysdeps/unix/sysv/linux/sendfile.c
>> +++ b/sysdeps/unix/sysv/linux/sendfile.c
>> @@ -32,7 +32,7 @@ sendfile (int out_fd, int in_fd, off_t *offset, size_t count)
>>    return INLINE_SYSCALL_CALL (sendfile, out_fd, in_fd, offset, count);
>>  # else
>>    __off64_t off64;
>> -  int rc;
>> +  ssize_t rc;
>>  
>>    if (offset != NULL)
>>      {
>>
> 
> This one looks wrong but all arches have __NR_sendfile, so limited impact.

$ python3 sysdeps/unix/sysv/linux/glibcsyscalls.py query-syscall sendfile
sendfile:
  defined: aarch64 alpha arm hppa i386 loongarch m68k microblaze mips/mips32 mips/mips64/n32 mips/mips64/n64 powerpc/powerpc32 powerpc/powerpc64 riscv/rv64 s390/s390-32 s390/s390-64 sh sparc/sparc32 sparc/sparc64 x86_64/64 x86_64/x32
  undefined: arc csky or1k riscv/rv32

The arc, ork1, and riscv/v32 has __OFF_T_MATCHES_OFF64_T, but even on or1k
it should not be a problem because off_t will be the size of int.

> 
>>
>> (Completely untested, it's just from the audit I did earlier.)
>>
>>> [...]
>>
>> sam
> 
> sam

So I think for INLINE_SYSCALL_CALL we are currently fine, although I agree
that 'long int' is the correct type for the return value.