netdev - Re: [PATCH v3] 3c59x: Fix memory leaks in vortex

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]

Date:	Tue, 23 Dec 2014 23:00:01 +0800
From:	Jia-Ju Bai <baijiaju1990@....com>
To:	Neil Horman <nhorman@...driver.com>
CC:	davem@...emloft.net, ebiederm@...ssion.com,
	dingtianhong@...wei.com, paul.gortmaker@...driver.com,
	justinvanwijngaarden@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH v3] 3c59x: Fix memory leaks in vortex_open

Thanks for the reply!

> This doesn't make sense.  We free all the skbs in vortex_open if we don't
> allocate all of them (the if (i != RX_RING_SIZE) check), the only place we miss
> is if vortex_up fails, and you didn't remove the if (!retval) goto out check, so
> this code won't get run appropriately.

In the code, when vortex_up is failed and does not returns 0,
"if (!retval)" is failed and "goto out" is not executed, so error 
handling code
below is executed, including my added code.
Is it right?

>
> That said, it does seem we need to clean up if vortex_up fails, but it would
> seem to me to be easier to just call vortex_close if it does, since that will do
> all of the approriate cleanup.
>
> Neil
>

In the code, vortex_close does too many releasing operations, such as free
vp->tx_skbuff, but vortex_open only allocates memory for vp->rx_skbuff 
before
vortex_up is called.
So I think it is enough to just free the memory of vp->rx_skbuff when 
vortex_up
is failed.

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