338453 - Leaks in security/nss/lib/jar/jarfile.c

Status: RESOLVED FIXED

(NSS :: Tools, defect, P3)

Description

[Kenneth Herron]

Please refer to the sample URL. Coverity CID 565 is that the |outbuf| allocation at line 366 is leaked at lines 376 and 391. It also appears to me that line 414 is another leak site, and the |inbuf| allocation at line 363 is also leaked in these cases.

Coverity 563 is that the |outbuf| allocation at line 468 is leaked at line 480. It appears to me that lines 494 and 502 also leak.

Comment 1

[(no longer active)]

Attachment: Prevent memory leak

I fixed the leaks in jar_physical_inflate() and jar_inflate_memory().

There are other leaks in this file.  Should I open new bugs and submit patches under those, or fix them all and post a new patch here?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 236777 [details] [diff] [review]
Prevent memory leak

r=nelsonb

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Ehasn,  Thanks for your patch!

There are other bugs already filed for leaks in lib/jar.
See bug 334914, bug 337354 and bug 337361 

If you find additional leaks in those files, not covered by those bug reports,
please add comments to those bugs documenting the additional leaks.  
Not much point in having multiple bugs filed for leaks in the same source
file.  Feel free to write patches and attach them to any of those bugs. 
You may also "take" those bug (reassign them to yourself).

Checkins of the approved patches must be done by NSS team members, but 
they'll happily give you credit in the checkin messages.  

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Ehsan,  sorry for mistyping your name.  

Comment 5

[(no longer active)]

Attachment: Prevent more memory leaks in jarfile.c

(In reply to comment #3)
> Ehasn,  Thanks for your patch!
> 
> There are other bugs already filed for leaks in lib/jar.
> See bug 334914, bug 337354 and bug 337361 

I'll take a look at those bugs as well.  Thanks!

> If you find additional leaks in those files, not covered by those bug reports,
> please add comments to those bugs documenting the additional leaks.  
> Not much point in having multiple bugs filed for leaks in the same source
> file.  Feel free to write patches and attach them to any of those bugs. 
> You may also "take" those bug (reassign them to yourself).

There is one more leak in jarfile.c, so I'm posting a new attachment which deprecates attachment #236777 [details] [diff] [review].  This new patch includes the previous, as well as this other leak's fix.

(In reply to comment #4)
> Ehsan,  sorry for mistyping your name.  

No worries.  I'm used to the "Eshan" spelling variant as well.  ;-)

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 236779 [details] [diff] [review]
Prevent more memory leaks in jarfile.c

r=nelsonb, requesting second review for checkin.

Comment 7

[Alexei Volkov]

Comment on attachment 236779 [details] [diff] [review]
Prevent more memory leaks in jarfile.c

in jar_physical_inflate, JAR_FOPEN opens a file descriptor(jarfile.c:378), and leaks it upon a return from the function with an error.

Comment 8

[(no longer active)]

Attachment: Revision of attachment #236779

(In reply to comment #7)
> (From update of attachment 236779 [details] [diff] [review] [edit])
> in jar_physical_inflate, JAR_FOPEN opens a file descriptor(jarfile.c:378), and
> leaks it upon a return from the function with an error.

That's right.  Sorry I didn't catch it at first.  This patch fixes this issue.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 236820 [details] [diff] [review]
Revision of attachment #236779 [details] [diff] [review]

Alexei, thanks for catching that FD leak.

r=nelsonb
My review+ signifies that this patch correctly fixes the leaks it is intended to fix, not that if fixes all possible leaks in this code.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Fix leaks in jarfile.c (bug 338453), jarjart.c (bug 351408), and
jarver.c (bug 337361). Patch contributed by [e-mail]

Checking in jarfile.c; new revision: 1.6; previous revision: 1.5
Checking in jarjart.c; new revision: 1.7; previous revision: 1.6
Checking in jarver.c;  new revision: 1.12; previous revision: 1.11