wild use of strlcpy() broke ipv6 support

[Jelmer Vernooij]

Am 01/04/12 10:07, schrieb Matthieu Patou:
> On 03/30/2012 09:21 PM, Jeremy Allison wrote:
>> On Fri, Mar 30, 2012 at 08:53:21PM -0700, Jeremy Allison wrote:
>>> On Fri, Mar 30, 2012 at 08:52:09PM -0700, Jeremy Allison wrote:
>>>> On Fri, Mar 30, 2012 at 06:02:06PM -0700, Matthieu Patou wrote:
>>>>> At least for this two changes you didn't get completely the sense of
>>>>> the strlcpy(), the idea is that if you have
>>>>> fe80::221:ccff:fe5f:7e51%eth0 to get the number of the interface and
>>>>> remove what is after '%'.
>>>>> so we should in this case always truncate and ihmo it's not a
>>>>> problem.
>>>> Ok - I'm boilerplate fixing all strlcpy uses. If it truncates
>>>> then by definition it's a problem, and we shouldn't be using
>>>> strlcpy() - we should be correctly paying attention to
>>>> the length we really want.
>>>>
>>>> I do (vaguely :-) remember writing this code (or something
>>>> like it). I'll take a look and fix up asap.
>>> Ah yes - now I looked at it I did write this code originally :-).
>>>
>>> I'll fix.
>> Here's the fix (attached). autobuilding now.
>>
>> Sorry for the breakage, but we really do need to
>> correctly use strlcpy/strlcat to check if we ever
>> get truncation (I'm at fault here, as I introduced
>> them originally - and added the incorrect idiom
>> when using them).
>>
>> I'll try and be a little more careful when adding
>> the checks if we're doing anything sophisticated
>> with the lengths. It's a very rare case where
>> we're using truncation as a desired side effect
>> (and is an *explicitly* incorrect use of the API :-).
> I'll just tested your patch it's working for local-link ipv6, was too
> busy today between homedepot and fry's and enjoying this beautifull
> bay area weather :-)
> And please can you recheck that you didn't introduced other regression.
>
> I'm thinking of having one instance of samba in make test with
> interface + bind interface only combo set so that we can catch this
> problem in the future.
That might be useful, the first step really should be to have more unit
tests for these parts of the code with IPv6.

Increasing the number of environments that the testsuite runs against
doesn't scale well, we should really try to avoid that if we at all can.

Cheers,

Jelmer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120402/11e6f152/attachment.pgp>