netdev - Re: [PATCH net] ipvlan: fix addr hash list corruption

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: <CAF2d9jh+VozezamrEK_6vfTvY8LZSzw9n-bJjSX3G11RXmfz3w@mail.gmail.com>
Date:	Tue, 24 Mar 2015 16:16:38 -0700
From:	Mahesh Bandewar <maheshb@...gle.com>
To:	Jiri Benc <jbenc@...hat.com>
Cc:	David Miller <davem@...emloft.net>,
	linux-netdev <netdev@...r.kernel.org>, dcbw@...hat.com
Subject: Re: [PATCH net] ipvlan: fix addr hash list corruption

On Tue, Mar 24, 2015 at 10:06 AM, Jiri Benc <jbenc@...hat.com> wrote:
> On Tue, 24 Mar 2015 13:00:37 -0400 (EDT), David Miller wrote:
>> From: Jiri Benc <jbenc@...hat.com>
>> Date: Mon, 23 Mar 2015 22:10:19 +0100
>>
>> > @@ -504,7 +504,8 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
>> >
>> >     if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
>> >             list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
>> > -                   ipvlan_ht_addr_del(addr, !dev->dismantle);
>> > +                   if (netif_running(dev))
>> > +                           ipvlan_ht_addr_del(addr, !dev->dismantle);
>> >                     list_del_rcu(&addr->anode);
>> >             }
>> >     }
>>
>> This is so error prone, because you are depending upon so many implementation
>> details to infer a boolean state "is this address hashed".
>>
>> So just add the boolean state to struct ipvl_addr, and manage it in
>> the ipvlan_ht_addr_{add,del}() code.
>
> I had that originally but then decided to go with smaller memory
> footprint. Which obviously doesn't matter much here. I'll send v2 with
> the boolean state.
>
Hi Jiri,

Well, we already have hlist_unhashed().The following patch should fix
the duplicate addition as well as deletion. Please give it a try.

diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 2a175006028b..8a542b9340c4 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -81,12 +81,13 @@ void ipvlan_ht_addr_add(struct ipvl_dev *ipvlan,
struct ipvl_addr *addr)
        hash = (addr->atype == IPVL_IPV6) ?
               ipvlan_get_v6_hash(&addr->ip6addr) :
               ipvlan_get_v4_hash(&addr->ip4addr);
-       hlist_add_head_rcu(&addr->hlnode, &port->hlhead[hash]);
+       if (hlist_unhashed(&addr->hlnode))
+               hlist_add_head_rcu(&addr->hlnode, &port->hlhead[hash]);
 }

 void ipvlan_ht_addr_del(struct ipvl_addr *addr, bool sync)
 {
-       hlist_del_rcu(&addr->hlnode);
+       hlist_del_init_rcu(&addr->hlnode);
        if (sync)
                synchronize_rcu();
 }

Thanks,
--mahesh..
>  Jiri
>
> --
> Jiri Benc
--
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