From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Perches Subject: Re: [PATCH] Fix infinite loop on dev_mc_unsync() Date: Fri, 09 Nov 2007 16:08:40 -0800 Message-ID: <1194653320.19522.28.camel@localhost> References: <20071109151135.GA12982@pogo> <20071109183733.GA22714@pogo> <1194635236.19522.3.camel@localhost> <20071109192033.GB22714@pogo> <4734E962.3010603@trash.net> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: "Luis R. Rodriguez" , netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jeff Garzik , David Miller To: Patrick McHardy Return-path: In-Reply-To: <4734E962.3010603-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org> Sender: linux-wireless-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: netdev.vger.kernel.org On Sat, 2007-11-10 at 00:12 +0100, Patrick McHardy wrote: > This may cause a use-after-free since __dev_addr_delete frees the address > when all references are gone. How about a comment then? Perhaps: diff --git a/net/core/dev_mcast.c b/net/core/dev_mcast.c index ae35405..63576aa 100644 --- a/net/core/dev_mcast.c +++ b/net/core/dev_mcast.c @@ -165,16 +165,23 @@ void dev_mc_unsync(struct net_device *to, struct net_device *from) netif_tx_lock_bh(from); netif_tx_lock_bh(to); + /* + This while loop can't be written as + for (da = from->mc_list; da; da = da->next) + da = from->mc_list and __dev_addr_delete can kfree(from->mc_list) + which could cause a use-after-free of da->next + */ + da = from->mc_list; while (da != NULL) { next = da->next; - if (!da->da_synced) - continue; - __dev_addr_delete(&to->mc_list, &to->mc_count, - da->da_addr, da->da_addrlen, 0); - da->da_synced = 0; - __dev_addr_delete(&from->mc_list, &from->mc_count, - da->da_addr, da->da_addrlen, 0); + if (da->da_synced) { + __dev_addr_delete(&to->mc_list, &to->mc_count, + da->da_addr, da->da_addrlen, 0); + da->da_synced = 0; + __dev_addr_delete(&from->mc_list, &from->mc_count, + da->da_addr, da->da_addrlen, 0); + } da = next; } __dev_set_rx_mode(to);