From mboxrd@z Thu Jan  1 00:00:00 1970
From: Valdis.Kletnieks@vt.edu (Valdis.Kletnieks at vt.edu)
Date: Wed, 13 Aug 2014 01:35:24 -0400
Subject: [PATCH] staging: Check for Null return of allocated skb in
	fw_download_code
In-Reply-To: Your message of "Tue, 12 Aug 2014 23:24:32 -0400."
	<1407900272-29402-1-git-send-email-xerofoify@gmail.com>
References: <1407900272-29402-1-git-send-email-xerofoify@gmail.com>
Message-ID: <14218.1407908124@turing-police.cc.vt.edu>
To: kernelnewbies@lists.kernelnewbies.org
List-Id: kernelnewbies.lists.kernelnewbies.org

On Tue, 12 Aug 2014 23:24:32 -0400, Nicholas Krause said:
> This patch checks if we are getting a Null allocated skb in the while/do
> loop of this function.
>  		skb  = dev_alloc_skb(frag_length + 4);
> +		if (skb == NULL) {
> +			rt_status =  false;
> +			break;
> +		}

Nick, it's *STILL* wrong.  And although I admit I blew it on the refcount
issue, that doesn't change the fact that you're still leaking memory here.

Work this through.  Hand simulate it.  Pretend that 3 fragments are
needed, and that the first two succeed but the third one fails.

Who frees the first two fragments before you return?

For bonus points - explain under what conditions this bug can *possibly*
be triggered on an actual system.  Consider in your reply both (a) when this
code is called and (b) what the system state *should* be at that point, and
what it has to be for this bug to trigger.  Given your answer to (a) and (b),
extrapolate to what the next few hundredths of a second likely hold for
this system even after we've fixed the bug you're mis-fixing here.

Once you've done that, you'll hopefully understand why you're trying to fix
a bug from 2009.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140813/5b0c1af4/attachment.bin