On 27 November 2012 13:27, Vineet Gupta <[email protected]> wrote:
> On Tuesday 27 November 2012 03:47 PM, Markos Chandras wrote:
>> On 26 November 2012 20:32, Rich Felker <[email protected]> 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.
>
>
Yeah I know (also wrote that down on the comment above the if{} block)
that this check does not prevent bad things
from happening. I will certainly remove it in my git repo ( I also
used it in other places as well ).
--
Regards,
Markos
_______________________________________________
uClibc mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/uclibc
