[quagga-dev 3075] Re: bugs related to length of interface names

Paul Jakma paul at clubi.ie
Fri Apr 1 18:38:19 BST 2005


On Fri, 1 Apr 2005, Andrew J. Schorr wrote:

> In my opinion, those two interface names do not match.  Do you feel 
> that they should match?  The problem is that the comparison code 
> silently truncates the string being searched for.  This sort of 
> unexpected behavior seems like bad API design to me.

Ah, ok, yes. Agreed - we shouldnt accept the overlong name in the 
first place.

> It also exposes us to possible bugs where the string being searched 
> for is actually a counted string of length IFNAMSIZ (not 
> INTERFACE_NAMSIZ), so we may run off the end by mistake.

Ok.

> the "n" somewhere else in the name?  Perhaps if_lookup_by_name_len is most
> transparent...

Maybe, I have no strong feelings ok.

> Yes, it does matter.  The issue is not the string inside the struct 
> interface. The potential bugs relate to the string being searched 
> for, which could be longer than INTERFACE_NAMSIZ (if passed in from 
> a config file or command line), or shorter (if coming from kernel).

Ok, but that would not be a matter of not accepting overlong string 
in the first place? If you do that then ifname as "nul terminated 
string up to INTERFACE_NAMSIZ bytes in size, not including the nul" 
is consistent no?

The nul will always be there, at or before offset INTERFACE_NAMSIZ.

> If the caller doesn't know the length of the string, then there's 
> no hope. :-)  Who else could know?  If the interface name is coming 
> from the kernel, then the proper call might be:
>
>   if_lookup_by_name_len(name, strnlen(name, IFNAMSIZ))

Hmm..

> But if it is coming from the command line or a config file, the proper
> call is (since there is no limit on the string length):
>
>   if_lookup_by_name_len(name, strlen(name))

So do the check when validating the user supplied name. After that we 
dont care, see the constraint above. No?

> And if it is coming through the zserv internal protocol, the proper call is:
>
>   if_lookup_by_name_len(name, strnlen(name, INTERFACE_NAMSIZ))
>
> Does that make sense?  There are 2 goals here: 1. avoid false 
> matches (where the library silently truncates the string being 
> searched for); and 2. avoid running off the end of a buffer that 
> may only be IFNAMSIZ bytes in length.

Ah..

But if INTERFACE_NAMSIZ is always >= IFNAMSIZ, and interface name is 
always stored in a INTERFACE_NAMSIZ+1 array, always nul-terminated, 
why bother exactly?

Or am i being dense? :)

regards,
-- 
Paul Jakma	paul at clubi.ie	paul at jakma.org	Key ID: 64A2FF6A
Fortune:



More information about the Quagga-dev mailing list