Re: RFC: libkern version of inet_ntoa_r

Hi,

On Sat, Jul 28, 2012 at 6:44 PM, Bjoern A. Zeeb
<bzeeb-lists_at_lists.zabbadoz.net> wrote:
> On Sat, 28 Jul 2012, Arnaud Lacombe wrote:
>
>> Hi,
>>
>> On Sat, Jul 28, 2012 at 6:14 PM, Bjoern A. Zeeb
>> <bzeeb-lists_at_lists.zabbadoz.net> wrote:
>>>
>>> On Wed, 25 Jul 2012, Luigi Rizzo wrote:
>>>
>>>> During some ipfw/dummynet cleanup i noticed that the libkern version of
>>>> inet_ntoa_r() is missing the buffer size argument that is present in
>>>> the libc counterpart.
>>>>
>>>> Any objection if i fix it ?
>>>
>>>
>>>
>>> And why exactly would you need it?  What does libc do with it?  Render
>>> partial IPv4 addresses?
>>>
>> Mitigate possibilities of memory corruption ? At the very least, allow
>> the following:
>>
>> {
>>    char tmp[sizeof "255.255.255.255"];
> char tmp[INET_ADDRSTRLEN];
>
Should I mention that this is taken directly from
`lib/libc/inet/inet_ntop.c' ? you may want to fix it, as you have been
blessed with a commit bit.

>>
>>    KASSERT(size >= (sizeof tmp));
>
> This would need to go into the called library function and cannot.
>
> So that gives you what extra checking exactly?  That the programmer got
> the sizeof right rather than the buffer size? You pushed some more on the
> stack or reused an register
That is funny. I was told that 2 always unused extra argument all
along the mutex API could not change anything, and now I am told the
opposite for 1 argument. There is no point trading the cost of a
register for overall runtime correctness. This is a string
manipulation function, it must be doing some kind of size check.

> for something that is supposed to be at a  minial fixed length
>
"supposed to be" ? you do not seem to be really sure to know how
inet_ntoa_r() is gonna be used, nor have you any way, by your words,
enforce it in any way.

> (nothing else lower allowed and will ever result
> in anything but misbehaviour) no matter what.
>
I'd be more than happy to welcome you tracking down memory corruption
based misbehaviors, but I prefer to detect it before, than attempt to
catch it after, it happens.

> It's not like it's
> inet_pton which can take totally different sizes.
>
that's nothing but an implementation details.

>
> Which again leaves me with the question - why does libc have it?
>
It is passed down to strlcpy(). You could have found this out by
yourself in a minute or so... But again, implementation details, might
they be incomplete, are irrelevant.

 - Arnaud