[PATCH v2 28/46] statfs: Use statfs64 if arch does not have the statfs syscall

[Vineet Gupta]

On Tuesday 27 November 2012 03:47 PM, Markos Chandras wrote:
> On 26 November 2012 20:32, Rich Felker <dalias at aerifal.cx> wrote:
>>
>> As far as I know, there's nothing to maintain "backwards
>> compatibility" with. The stat structures vary per-arch, and an arch
>> that doesn't have a non-64-bit statfs syscall doesn't have an existing
>> "struct statfs" to maintain backwards compatibility with. You can just
>> define "struct statfs" to match the layout of the 64-bit struct.
> 
> I see what you mean but looking at the uClibc code
> (include/bits/statfs.h) struct statfs is always
> defined so your proposal of introducing yet another statfs for arches
> that don't have the non-64bit
> syscalls will conflict with this definition. It's always possible to
> #if/#endif the existing statfs structure with a new one
> for architectures that don't have the 32-bit version, but I don't
> think this is prettier to what I've done here.
> Same thing happens in the the kernel. A statfs structure is always
> exported whether you can support the
> 32-bit syscall or not. Also existing applications expect this
> well-known struct statfs. Changing it to a new one will
> result in an ABI break.
> 
>>
>> By the way, the old small-off_t functions are supposed to give an
>> error if any off_t value does not fit in the range of off_t, rather
>> than silently truncating. So even if you want to keep the conversion
>> code, you need to check this. If using my approach (with same struct
>> layout and padding for the extra 32 bits), you'd need to test that the
>> padding is zero and return an error if any nonzero value appeared in
>> the padding (except perhaps in the case of -1?).
>>
>>> As for the simple "buf" pointer check, I think you missed the point.
>>> This pointer is never passed to the kernel
>>> (it is not passed to the INLINE_SYSCALL macro) so the kernel does not
>>> know about it. It is possible your userspace
>>> program to pass an invalid "buf" pointer and the INLINE_SYSCALL to
>>> succeed but you will crash after that when you try
>>> to dereference any member of the invalid "buf" pointer.
>>
>> Crashing is perfectly acceptable behavior. statfs() is under no
>> obligation to report EFAULT; passing an invalid pointer is undefined
>> behavior and the expected result should be a crash or worse. Even if
>> the kernel handles it, passing bogus pointers won't necessarily give
>> EFAULT; if they happen to point to writable memory, you'll just
>> clobber random memory.
>>
>> Rich
> 
> Well yeah, it just feels weird for your userspace application to be
> able to crash your C library,
> although, as you correctly pointed out, it will not handle bad
> possitive pointers.
> Normally the kernel would handle bad user input but in this case the
> kernel has no
> idea about it.
> 

But libc might not necessarily have the right idea either - if we pay
attention to Mark's earlier comment on thread the whole discussion
becomes moot since the check is NOT portable hence can't be taken in
independnet of it's value on some arches.