[PATCH net-next] net: fix address check in rtnl_fdb_del

[PATCH net-next] net: fix address check in rtnl_fdb_del

[Vlad Yasevich]

Commit 6681712d67eef14c4ce793561c3231659153a320
	vxlan: generalize forwarding tables

relaxed the address checks in rtnl_fdb_del() to use is_zero_ether_addr().
This allows users to add multicast addresses using the fdb API.  However,
the check in rtnl_fdb_del() still uses a more strict
is_valid_ether_addr() which rejects multicast addresses.  Thus it
is possible to add an fdb that can not be later removed.
Relax the check in rtnl_fdb_del() as well.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 net/core/rtnetlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 18af08a..2c54cc1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2192,7 +2192,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh)
 	}
 
 	addr = nla_data(tb[NDA_LLADDR]);
-	if (!is_valid_ether_addr(addr)) {
+	if (!is_zero_ether_addr(addr)) {
 		pr_info("PF_BRIDGE: RTM_DELNEIGH with invalid ether address\n");
 		return -EINVAL;
 	}
-- 
1.8.1.4

Re: [PATCH net-next] net: fix address check in rtnl_fdb_del

[Ben Hutchings]

On Tue, 2013-04-23 at 16:39 -0400, Vlad Yasevich wrote:
> Commit 6681712d67eef14c4ce793561c3231659153a320
> 	vxlan: generalize forwarding tables
> 
> relaxed the address checks in rtnl_fdb_del() to use is_zero_ether_addr().
> This allows users to add multicast addresses using the fdb API.  However,
> the check in rtnl_fdb_del() still uses a more strict
> is_valid_ether_addr() which rejects multicast addresses.  Thus it
> is possible to add an fdb that can not be later removed.
> Relax the check in rtnl_fdb_del() as well.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  net/core/rtnetlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 18af08a..2c54cc1 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2192,7 +2192,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh)
>  	}
>  
>  	addr = nla_data(tb[NDA_LLADDR]);
> -	if (!is_valid_ether_addr(addr)) {
> +	if (!is_zero_ether_addr(addr)) {

This is the opposite of what you want.

Ben.

>  		pr_info("PF_BRIDGE: RTM_DELNEIGH with invalid ether address\n");
>  		return -EINVAL;
>  	}

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net-next] net: fix address check in rtnl_fdb_del

[Vlad Yasevich]

On 04/23/2013 04:47 PM, Ben Hutchings wrote:
> On Tue, 2013-04-23 at 16:39 -0400, Vlad Yasevich wrote:
>> Commit 6681712d67eef14c4ce793561c3231659153a320
>> 	vxlan: generalize forwarding tables
>>
>> relaxed the address checks in rtnl_fdb_del() to use is_zero_ether_addr().
>> This allows users to add multicast addresses using the fdb API.  However,
>> the check in rtnl_fdb_del() still uses a more strict
>> is_valid_ether_addr() which rejects multicast addresses.  Thus it
>> is possible to add an fdb that can not be later removed.
>> Relax the check in rtnl_fdb_del() as well.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>>   net/core/rtnetlink.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 18af08a..2c54cc1 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -2192,7 +2192,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh)
>>   	}
>>
>>   	addr = nla_data(tb[NDA_LLADDR]);
>> -	if (!is_valid_ether_addr(addr)) {
>> +	if (!is_zero_ether_addr(addr)) {
>
> This is the opposite of what you want.

of course you are right....  totally forgot the '!'...

Thanks
-vlad
>
> Ben.
>
>>   		pr_info("PF_BRIDGE: RTM_DELNEIGH with invalid ether address\n");
>>   		return -EINVAL;
>>   	}
>

Re: [PATCH net-next] net: fix address check in rtnl_fdb_del

[David Miller]

From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 23 Apr 2013 17:04:46 -0400

> On 04/23/2013 04:47 PM, Ben Hutchings wrote:
>> On Tue, 2013-04-23 at 16:39 -0400, Vlad Yasevich wrote:
>>> @@ -2192,7 +2192,7 @@ static int rtnl_fdb_del(struct sk_buff *skb,
>>> struct nlmsghdr *nlh)
>>>   	}
>>>
>>>   	addr = nla_data(tb[NDA_LLADDR]);
>>> -	if (!is_valid_ether_addr(addr)) {
>>> +	if (!is_zero_ether_addr(addr)) {
>>
>> This is the opposite of what you want.
> 
> of course you are right....  totally forgot the '!'...

Please test your patches.

If this patch is so unimportant that you don't feel inclined to test
it, it's probably not important enough for me to apply either.