Re: [PATCH] Fix infinite loop on dev_mc_unsync()

[Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>]

Joe Perches wrote:
> 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
> +	*/
>   

Seems unnecessary to me, we also don't comment each list_for_each_entry_safe
iteration. I consider the use of a seperate next variable self-explanatory.