netdev - Re: [PATCH iproute2-next] ipaddress: fix label matching

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <057043f9-e476-cde8-4915-8e350fc6d848@gmail.com>
Date:   Sat, 14 Jul 2018 11:27:37 +0300
From:   Serhey Popovych <serhe.popovych@...il.com>
To:     Vincent Bernat <vincent@...nat.im>, David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, stephen@...workplumber.org
Subject: Re: [PATCH iproute2-next] ipaddress: fix label matching

Vincent Bernat wrote:
>  ❦ 11 juillet 2018 21:01 -0400, David Ahern <dsahern@...il.com> :
> 
>>> +++ b/ip/ipaddress.c
>>> @@ -837,11 +837,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
>>>  	if (!name)
>>>  		return -1;
>>>  
>>> -	if (filter.label &&
>>> -	    (!filter.family || filter.family == AF_PACKET) &&
>>> -	    fnmatch(filter.label, name, 0))
>>> -		return -1;
>>> -
>>
>> The offending commit changed the return code:
>>
>>         if (filter.label &&
>>             (!filter.family || filter.family == AF_PACKET) &&
>> -           fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
>> -               return 0;
>> +           fnmatch(filter.label, name, 0))
>> +               return -1;
>>
>>
>> Vincent: can you try leaving the code as is, but change the return to 0?
> 
> Yes, it works by just returning 0. The code still doesn't make sense.
> 

I think return code is correct. Check presented by this code too because
print_linkinfo() isn't static and called from ipmonitor.c where no
ipaddr_filter() or similar call that filters by label present.

Instead fnmatch() compares interface *name*, not label from IFA_LABEL
attribute. Thus:

    fnmatch(pattern, string, flags) ->
    fnmatch("lo:1", "lo", 0) == FNM_NOMATCH (1)

Assuming above I would like to see ifa_label_match_rta() instead of open
coded checks for filter.label with fmatch() in print_linkinfo().

Also it might be good idea to pass @name from get_ifname_rta() (like we
do in print_linkinfo()) to ifa_label_match_rta() so that we respect
IFLA_IFNAME if present.



Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)