netdev - Re: [PATCH 3/3] dsa: fix freeing of sparse port allocation

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] [day] [month] [year] [list]

Message-Id: <201303252055.33840.florian@openwrt.org>
Date:	Mon, 25 Mar 2013 20:55:33 +0100
From:	Florian Fainelli <florian@...nwrt.org>
To:	"David Laight" <David.Laight@...lab.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH 3/3] dsa: fix freeing of sparse port allocation

Le lundi 25 mars 2013 17:03:23, David Laight a écrit :
> > If we have defined a sparse port allocation which is non-contiguous and
> > contains gaps, the code freeing port_names will just stop when it
> > encouters a first NULL port_names, which is not right, we should iterate
> > over all possible number of ports (DSA_MAX_PORTS) until we are done.
> 
> ...
> 
> >  		port_index = 0;
> > 
> > -		while (pd->chip[i].port_names &&
> > -			pd->chip[i].port_names[++port_index])
> > -			kfree(pd->chip[i].port_names[port_index]);
> > +		while (port_index < DSA_MAX_PORTS) {
> > +			if (pd->chip[i].port_names[port_index])
> > +				kfree(pd->chip[i].port_names[port_index]);
> > +			port_index++;
> > +		}
> 
> A couple of 'odd' differences:
> 
> The old code checked pd->chip[i].port_names (as if it
> might be a pointer) that is absent from the new version.
> (If it is separately allocated it is leaked).
> 
> The new code tests and frees pd->chip[i].port_names[0]
> whereas the old code ignored the 0th entry.

The old code was wrong, it was off by one for the first array index, and would 
stop whenever it encountered the first NULL port_names[index] so we would not 
skip other these and free the possibly non-NULL next one.

I think the current code is now correct, but thanks for the review!
--
Florian
--
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