[lkml]   [2016]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] net: rtnl: avoid uninitialized data in IFLA_VF_VLAN_LIST handling
Hi Arnd,

On 30/09/2016 7:38 PM, Arnd Bergmann wrote:
> On Friday 30 September 2016, Eric Dumazet wrote:
>>> @@ -1753,6 +1753,9 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
>>> len++;
>>> }
>>> + if (len == 0)
>>> + return -EINVAL;
>>> +
>>> err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
>>> ivvl[0]->qos, ivvl[0]->vlan_proto);
>>> if (err < 0)
>>> --
>>> 2.9.0
Reviewed-by: Moshe Shemesh <>

Thanks for the fix, I will add the flag to my makefile.
>> So, if I read this code, we build an array, but call ndo_set_vf_vlan()
>> only using first element ?
>> Looks like the bug should be fixed in a different way.
> I was wondering about this too, but didn't understand enough about it to say
> if it was intentional or not.
It is intentional.
The change we need here is the option to set the VF vlan protocol, so
our hw will insert either S-TAG or C-TAG per VF. The previous version of
this patch didn't use any array.
Then we got a comment, which makes sense, that the new UAPI interface
should be able to support a future driver that might want to set both
S-TAG and C-TAG per VF.
We could have used a single struct on kernel side instead of a list of
size 1, but this would cause bad/not scalable code in the parsing stage.
As the UAPI is a list, in the parsing stage we thought it is more
correct to use the constant MAX_VLAN_LIST_LEN and iterate as of a
general case, even though currently the list size is one.

> I just realized that I forgot to add Moshe and Tariq
> on Cc (I relied on scripts/, but didn't double-check).
> I've added them to Cc now, hope they can clarify this.
> Arnd

Moshe and Tariq

 \ /
  Last update: 2016-10-02 21:37    [W:0.045 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site