On Tue, 12 Aug 2014 16:19:40 -0400, Nick Krause said: > On Tue, Aug 12, 2014 at 4:18 PM, Nicholas Krause <xerofoify@xxxxxxxxx> wrote: > > I am fixing the bug entry , https://bugzilla.kernel.org/show_bug.cgi?id=60461. > > This entry states that we are not checking the skb allocated in fw_download_code > > for NULL and after checking it ,I fixed it to check for the NULL value before > > returning false and exiting fw_download_code cleanly. > I am trying to get this patch merged and after my issues with the > kernel community, I can't get this into the mainline. No, you're having trouble getting this into mainline because you are *STILL* persisting in submitting patches that are buggy. In this case, the problem is you *DON'T* exit the function cleanly. Note your patch causes an immediate return from inside a do/while loop, which *also* contains: skb_put(skb, i); So if there's (say) 3 fragments needed, and we fail on the allocation of the third one, you just leaked references to the first two fragments, and never actually clean up the allocations, so we have references to leaked memory. And leaking memory in a case where we're almost certainly very close to OOM isn't exactly a good idea. Yes, failing to check the return code is a bug - but so is failing to unwind the allocations already made. It took me all of a minute to spot this issue - the only clue needed was that there was a '*_put()' call in the function, which should be a warning flag that reference counting needs to be checked. Greg: Consider this a NACK of this patch. Nick: If you're going to fix this bug, *UNDERSTAND THE CODE* and fix it *CORRECTLY*. Seriously Nick. *PLEASE* stop posting patches until you've gotten a better handle on what code maintenance really entails.
Attachment:
pgpjEeBOutXxz.pgp
Description: PGP signature
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@xxxxxxxxxxxxxxxxx http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
