netdev - Re: getting VF link info seems to be broken in 3.9-rc8

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] [day] [month] [year] [list]

Message-ID: <51799BB6.5010807@intel.com>
Date:	Thu, 25 Apr 2013 14:10:14 -0700
From:	Alexander Duyck <alexander.h.duyck@...el.com>
To:	David Miller <davem@...emloft.net>
CC:	stephen@...workplumber.org, mitch.a.williams@...el.com,
	ogerlitz@...lanox.com, gregory.v.rose@...el.com,
	e1000-devel@...ts.sourceforge.net, netdev@...r.kernel.org,
	ronye@...lanox.com, shemminger@...tta.com
Subject: Re: getting VF link info seems to be broken in 3.9-rc8

On 04/25/2013 01:49 PM, David Miller wrote:
> From: Stephen Hemminger <stephen@...workplumber.org>
> Date: Thu, 25 Apr 2013 13:45:13 -0700
>
>> On Thu, 25 Apr 2013 13:36:06 -0700
>> Alexander Duyck <alexander.h.duyck@...el.com> wrote:
>>
>>> On 04/25/2013 01:25 PM, David Miller wrote:
>>>> From: Alexander Duyck <alexander.h.duyck@...el.com>
>>>> Date: Thu, 25 Apr 2013 13:20:24 -0700
>>>>
>>>>> On 04/25/2013 12:24 PM, David Miller wrote:
>>>>>> From: Alexander Duyck <alexander.h.duyck@...el.com>
>>>>>> Date: Thu, 25 Apr 2013 11:29:04 -0700
>>>>>>
>>>>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>>>>> index b65441d..23854b5 100644
>>>>>> --- a/net/core/rtnetlink.c
>>>>>> +++ b/net/core/rtnetlink.c
>>>>>> @@ -1072,7 +1072,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
>>>>>>  	rcu_read_lock();
>>>>>>  	cb->seq = net->dev_base_seq;
>>>>>>  
>>>>>> -	if (nlmsg_parse(cb->nlh, sizeof(struct rtgenmsg), tb, IFLA_MAX,
>>>>>> +	if (nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
>>>>>>  			ifla_policy) >= 0) {
>>>>>>  
>>>>>>  		if (tb[IFLA_EXT_MASK])
>>>>>> @@ -1922,7 +1922,7 @@ static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
>>>>>>  	u32 ext_filter_mask = 0;
>>>>>>  	u16 min_ifinfo_dump_size = 0;
>>>>>>  
>>>>>> -	if (nlmsg_parse(nlh, sizeof(struct rtgenmsg), tb, IFLA_MAX,
>>>>>> +	if (nlmsg_parse(nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
>>>>>>  			ifla_policy) >= 0) {
>>>>>>  		if (tb[IFLA_EXT_MASK])
>>>>>>  			ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
>>>>> I thought that as well.  I tried reverting it and the issue is still there.
>>>>>
>>>>> However, I do think this may be part of the issue since I added a printk
>>>>> to dump nlmsg_attrlen before going into the nlmsg_parse and with
>>>>> ifinfomsg the attrlen is -12, with rtgenmsg it is 0.
>>>> I wonder if we are seeing two ways tools are making these calls, some are
>>>> passing rtgenmsg and some are passing ifinfomsg.  The latter, I am mostly
>>>> convinced, is what we must see here from properly written applications.
>>>>
>>>> That would be really unfortunate, but seeing a nlmsg_attrlen() of -12 would
>>>> seem to confirm that a rtgenmsg was used.
>>>>
>>>> I guess you're using iproute2?
>>> Yes.  All I am doing is ip link show and previously this would display
>>> VF info as well as PF.  Now the VF info is not there.
>>>
>>> From what I can tell the problem call is rtnl_wilddump_req_filter since
>>> it is only passing a rtgenmsg and asking us to dump the link with the
>>> RTEXT_FILTER_VF filter mask.
>> It looks like a bug in the initial code for filter. in this case, it calls:
>>   ip_list_flush_or_save
>>      rtnl_wilddump_req_filter
>>        which sends 'struct rtgenmsg' 
>>
>> This was probably a mistake in the original flags addition, not sure if we can
>> fix it now that ABI is set. Probably have to revert the kernel change.
> But we know there are tools, just as widely deployed, passing in ifinfomsg.
> That's what trigged inclusion of the patch above in the first place.
>
> Let's just declare this an iproute2 bug, fix iproute2 to pass
> ifinfomsg too, and keep an eye out for when other folks run into this
> problem.

With the earlier patch reverted and the latest version iproute the
problem is resolved.  I'm suspecting the other issue may have been a
64/32 bit alignment issue since it seems like that was addressed
recently.  Odds are the message format for updated iproute will be
incompatible with legacy API anyway.

I will undo the revert and try tweaking the iproute2 call so that it
makes use of the ifinfomsg.  If that works okay I can send a patch.

Thanks,

Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html