Re: [PATCH] io: Return EBAFD for negative file descriptor on fstat (BZ #27559)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 12/03/2021 10:57, Adhemerval Zanella wrote:
> 
> 
> On 12/03/2021 10:44, Stefan Liebler wrote:
>> On 3/11/21 1:37 PM, Adhemerval Zanella via Libc-alpha wrote:
>>>
>>>
>>> On 11/03/2021 09:13, Florian Weimer wrote:
>>>> * Adhemerval Zanella via Libc-alpha:
>>>>
>>>>> Now that fstat is implemented on top fstatat we need to handle negative
>>>>> inputs.
>>>>
>>>> Please mention AT_FDCWD in the commit message.  Or add a comment to the
>>>> code that the check rejects AT_FDCWD, which would otherwise be accepted
>>>> by the kernel.
>>>>
>>>> The patch looks okay otherwise.
>>>
>>> Ack.  I will also add the missing tst-stat-lfs (which is just a wrapper
>>> to set __FILE_OFFSET_BITS=64).
>>>
>>
>> Hi Adhemerval,
>>
>> I've just recognized that io/tst-stat is failing on my s390 (31bit) system:
>> tst-stat.c:94: numeric comparison failure
>>
>>    left: 315117680 (0x12c85070); from: stx.stx_ctime.tv_nsec
>>
>>   right: 0 (0x0); from: st.st_ctim.tv_nsec
>>
>> tst-stat.c:96: numeric comparison failure
>>
>>    left: 315117680 (0x12c85070); from: stx.stx_mtime.tv_nsec
>>   right: 0 (0x0); from: st.st_mtim.tv_nsec
>> tst-stat.c:94: numeric comparison failure
>>    left: 315117680 (0x12c85070); from: stx.stx_ctime.tv_nsec
>>   right: 0 (0x0); from: st.st_ctim.tv_nsec
>> tst-stat.c:96: numeric comparison failure
>>    left: 315117680 (0x12c85070); from: stx.stx_mtime.tv_nsec
>>   right: 0 (0x0); from: st.st_mtim.tv_nsec
>> tst-stat.c:94: numeric comparison failure
>>    left: 315117680 (0x12c85070); from: stx.stx_ctime.tv_nsec
>>   right: 0 (0x0); from: st.st_ctim.tv_nsec
>> tst-stat.c:96: numeric comparison failure
>>    left: 315117680 (0x12c85070); from: stx.stx_mtime.tv_nsec
>>   right: 0 (0x0); from: st.st_mtim.tv_nsec
>> tst-stat.c:94: numeric comparison failure
>>    left: 315117680 (0x12c85070); from: stx.stx_ctime.tv_nsec
>>   right: 0 (0x0); from: st.st_ctim.tv_nsec
>> tst-stat.c:96: numeric comparison failure
>>    left: 315117680 (0x12c85070); from: stx.stx_mtime.tv_nsec
>>   right: 0 (0x0); from: st.st_mtim.tv_nsec
>> error: 8 test failures
>>
>>
>> But I have to admit, I haven't looked into the test yet and won't be
>> able before next week. But I wanted to report it now.
>>
>> Bye,
>> Stefan
> 
> 
> I will check this out on a s390 system.
> 

I think we stumbled on another kernel limitation/issue: the tst-stat uses
the __NR_fstatat64 call that for s390 will use the entrypoint:

arch/s390/kernel/compat_linux.c:

150 COMPAT_SYSCALL_DEFINE2(s390_stat64, const char __user *, filename, struct stat64_emu31 __user *, statbuf)
151 {
152         struct kstat stat;
153         int ret = vfs_stat(filename, &stat);
154         if (!ret)
155                 ret = cp_stat64(statbuf, &stat);
156         return ret;
157 }

And the cp_stat64 explicit omit the nanoseconds fields:

126 static int cp_stat64(struct stat64_emu31 __user *ubuf, struct kstat *stat)
127 {       
128         struct stat64_emu31 tmp;
129         
130         memset(&tmp, 0, sizeof(tmp));
131         
132         tmp.st_dev = huge_encode_dev(stat->dev);
133         tmp.st_ino = stat->ino;
134         tmp.__st_ino = (u32)stat->ino;
135         tmp.st_mode = stat->mode;
136         tmp.st_nlink = (unsigned int)stat->nlink;
137         tmp.st_uid = from_kuid_munged(current_user_ns(), stat->uid);
138         tmp.st_gid = from_kgid_munged(current_user_ns(), stat->gid);
139         tmp.st_rdev = huge_encode_dev(stat->rdev);
140         tmp.st_size = stat->size;
141         tmp.st_blksize = (u32)stat->blksize;
142         tmp.st_blocks = (u32)stat->blocks;
143         tmp.st_atime = (u32)stat->atime.tv_sec;
144         tmp.st_mtime = (u32)stat->mtime.tv_sec;
145         tmp.st_ctime = (u32)stat->ctime.tv_sec;
146         
147         return copy_to_user(ubuf,&tmp,sizeof(tmp)) ? -EFAULT : 0;
148 }

It seems deliberated from a comment that hits the idea was to in the
future to use the __pad[6,7,8] (meant to nanoseconds) as the high 
bits for the seconds part.

The straightforward fix would to just disable the nanoseconds check
for non-lfs interface.  For LFS interface s390 will use statx as
other architectures, which should mitigate the issue on recent
kernels.