Possible bug in getdents64()?

Possible bug in getdents64()?

[Alejandro Colomar]

The manual page for getdents64() says the prototype should be the following:

        int getdents64(unsigned int fd, struct linux_dirent64 *dirp,
                     unsigned int count);


Note the type of 'count': 'unsigned int'
(usually a 32-bit unsigned integer).
And the Linux kernel seems to use those types (fs/readdir.c:351):

SYSCALL_DEFINE3(getdents64, unsigned int, fd,
		struct linux_dirent64 __user *, dirent,
		unsigned int, count)
{
...
}

But glibc uses 'size_t' (usually a 64-bit unsigned integer)
for the parameter 'count' (sysdeps/unix/linux/getdents64.c:25):


/* The kernel struct linux_dirent64 matches the 'struct dirent64' type.  */
ssize_t
__getdents64 (int fd, void *buf, size_t nbytes)
{
   /* The system call takes an unsigned int argument, and some length
      checks in the kernel use an int type.  */
   if (nbytes > INT_MAX)
     nbytes = INT_MAX;
   return INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
}
libc_hidden_def (__getdents64)
weak_alias (__getdents64, getdents64)



Isn't it undefined behavior to pass a variable of a different (larger) 
type to a variadic function than what it expects?

Is that behavior defined in this implementation?

Wouldn't a cast to 'unsigned int' be needed?


Thanks,

Alex

Re: Possible bug in getdents64()?

[Alejandro Colomar]

[[ CC += linux-man, linux-kernel, libc-alpha, mtk ]]

On 2020-10-28 20:26, Alejandro Colomar wrote:
> The manual page for getdents64() says the prototype should be the 
> following:
> 
>         int getdents64(unsigned int fd, struct linux_dirent64 *dirp,
>                      unsigned int count);
> 
> 
> Note the type of 'count': 'unsigned int'
> (usually a 32-bit unsigned integer).
> And the Linux kernel seems to use those types (fs/readdir.c:351):
> 
> SYSCALL_DEFINE3(getdents64, unsigned int, fd,
>          struct linux_dirent64 __user *, dirent,
>          unsigned int, count)
> {
> ...
> }
> 
> But glibc uses 'size_t' (usually a 64-bit unsigned integer)
> for the parameter 'count' (sysdeps/unix/linux/getdents64.c:25):
> 
> 
> /* The kernel struct linux_dirent64 matches the 'struct dirent64' type.  */
> ssize_t
> __getdents64 (int fd, void *buf, size_t nbytes)
> {
>    /* The system call takes an unsigned int argument, and some length
>       checks in the kernel use an int type.  */
>    if (nbytes > INT_MAX)
>      nbytes = INT_MAX;
>    return INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
> }
> libc_hidden_def (__getdents64)
> weak_alias (__getdents64, getdents64)
> 
> 
> 
> Isn't it undefined behavior to pass a variable of a different (larger) 
> type to a variadic function than what it expects?
> 
> Is that behavior defined in this implementation?
> 
> Wouldn't a cast to 'unsigned int' be needed?
> 
> 
> Thanks,
> 
> Alex

Re: Possible bug in getdents64()?

[Florian Weimer]

* Alejandro Colomar via Libc-alpha:

> [[ CC += linux-man, linux-kernel, libc-alpha, mtk ]]
>
> On 2020-10-28 20:26, Alejandro Colomar wrote:
>> The manual page for getdents64() says the prototype should be the
>> following:
>>         int getdents64(unsigned int fd, struct linux_dirent64 *dirp,
>>                      unsigned int count);
>> 
>> Note the type of 'count': 'unsigned int'
>> (usually a 32-bit unsigned integer).
>> And the Linux kernel seems to use those types (fs/readdir.c:351):
>> SYSCALL_DEFINE3(getdents64, unsigned int, fd,
>>          struct linux_dirent64 __user *, dirent,
>>          unsigned int, count)
>> {
>> ...
>> }
>> But glibc uses 'size_t' (usually a 64-bit unsigned integer)
>> for the parameter 'count' (sysdeps/unix/linux/getdents64.c:25):
>> 
>> /* The kernel struct linux_dirent64 matches the 'struct dirent64' type.  */
>> ssize_t
>> __getdents64 (int fd, void *buf, size_t nbytes)
>> {
>>    /* The system call takes an unsigned int argument, and some length
>>       checks in the kernel use an int type.  */
>>    if (nbytes > INT_MAX)
>>      nbytes = INT_MAX;
>>    return INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
>> }
>> libc_hidden_def (__getdents64)
>> weak_alias (__getdents64, getdents64)
>> 
>> Isn't it undefined behavior to pass a variable of a different
>> (larger) type to a variadic function than what it expects?
>> Is that behavior defined in this implementation?
>> Wouldn't a cast to 'unsigned int' be needed?

There is no variadic function involved here.  INLINE_SYSCALL_CALL takes
care of the zero extension to the register internally, irrespective of
the argument type.  (The register is of type long int or long long int,
depending on the architecture.)

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: Possible bug in getdents64()?

[Alejandro Colomar]

On 2020-10-29 12:09, Florian Weimer wrote:
> * Alejandro Colomar via Libc-alpha:
> 
>> [[ CC += linux-man, linux-kernel, libc-alpha, mtk ]]
>>
>> On 2020-10-28 20:26, Alejandro Colomar wrote:
>>> The manual page for getdents64() says the prototype should be the
>>> following:
>>>          int getdents64(unsigned int fd, struct linux_dirent64 *dirp,
>>>                       unsigned int count);
>>>
>>> Note the type of 'count': 'unsigned int'
>>> (usually a 32-bit unsigned integer).
>>> And the Linux kernel seems to use those types (fs/readdir.c:351):
>>> SYSCALL_DEFINE3(getdents64, unsigned int, fd,
>>>           struct linux_dirent64 __user *, dirent,
>>>           unsigned int, count)
>>> {
>>> ...
>>> }
>>> But glibc uses 'size_t' (usually a 64-bit unsigned integer)
>>> for the parameter 'count' (sysdeps/unix/linux/getdents64.c:25):
>>>
>>> /* The kernel struct linux_dirent64 matches the 'struct dirent64' type.  */
>>> ssize_t
>>> __getdents64 (int fd, void *buf, size_t nbytes)
>>> {
>>>     /* The system call takes an unsigned int argument, and some length
>>>        checks in the kernel use an int type.  */
>>>     if (nbytes > INT_MAX)
>>>       nbytes = INT_MAX;
>>>     return INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
>>> }
>>> libc_hidden_def (__getdents64)
>>> weak_alias (__getdents64, getdents64)
>>>
>>> Isn't it undefined behavior to pass a variable of a different
>>> (larger) type to a variadic function than what it expects?
>>> Is that behavior defined in this implementation?
>>> Wouldn't a cast to 'unsigned int' be needed?
> 
> There is no variadic function involved here.  INLINE_SYSCALL_CALL takes
> care of the zero extension to the register internally, irrespective of
> the argument type.  (The register is of type long int or long long int,
> depending on the architecture.)

Hi Florian,

Thank you very much!

Alex

> 
> Thanks,
> Florian
>