Patchwork [PATCHv2,net-next] net: fix address check in rtnl_fdb_del

login
register
mail settings
Submitter Vlad Yasevich
Date April 23, 2013, 9:05 p.m.
Message ID <1366751123-10143-1-git-send-email-vyasevic@redhat.com>
Download mbox | patch
Permalink /patch/239011/
State Accepted
Delegated to: David Miller
Headers show

Comments

Vlad Yasevich - April 23, 2013, 9:05 p.m.
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(-)
David Miller - April 23, 2013, 10:30 p.m.
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 23 Apr 2013 17:05:23 -0400

> 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>

I don't think you were able to actually test this patch in the
amount of time between when the bug in your initial version was
shown to you and when you posted this new version.

If you indeed didn't test this patch, I really don't think that's
acceptable, to be honest with you.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vlad Yasevich - April 24, 2013, 12:29 a.m.
On 04/23/2013 06:30 PM, David Miller wrote:
> From: Vlad Yasevich <vyasevic@redhat.com>
> Date: Tue, 23 Apr 2013 17:05:23 -0400
>
>> 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>
>
> I don't think you were able to actually test this patch in the
> amount of time between when the bug in your initial version was
> shown to you and when you posted this new version.
>
> If you indeed didn't test this patch, I really don't think that's
> acceptable, to be honest with you.
>

I actually did test this patch and found the issue in the testing of the
bridging changes I was working on.  It just that I goofed the stand 
alone fix that I thought might warrant a separate submission while I
run more tests on the bridging code.

-vlad
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 25, 2013, 8:17 a.m.
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 23 Apr 2013 17:05:23 -0400

> 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>

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

Patch

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;
 	}