[CID 222162][CID 222161] overwrite_var: Overwriting subiter in subiter = Pk11Install_ListIter_new
RESOLVED
FIXED
in 3.22
Status
People
(Reporter: franziskus, Assigned: franziskus)
Tracking
(Blocks: 1 bug, {coverity})
Firefox Tracking Flags
(firefox44 affected)
Details
Attachments
(1 attachment, 1 obsolete attachment)
9.55 KB,
patch
|
mt
:
review+
|
Details | Diff | Splinter Review |
Coverity complains that subiter is overwritten here and thus leaks storage. > subiter = Pk11Install_ListIter_new(subpair->list); > [...] > Pk11Install_ListIter_delete(subiter); > subiter = NULL; I'm not sure what Pk11Install_ListIter_delete is supposed to do (delete probably), but it only sets > _this->list=NULL; > _this->current=NULL; without freeing anything. To make this work properly we have to free _this. We can also pull the subiter=NULL in the delete function.
(Assignee) | ||
Comment 1•3 years ago
|
||
Created attachment 8679460 [details] [diff] [review] don't leak the iter moved the free and null in the delete function
Attachment #8679460 -
Flags: review?(martin.thomson)
Comment 2•3 years ago
|
||
Comment on attachment 8679460 [details] [diff] [review] don't leak the iter Review of attachment 8679460 [details] [diff] [review]: ----------------------------------------------------------------- This might have been a double-free in the past, but removing the NULL assignments means that you have a new type of double-free. ::: cmd/modutil/install-ds.c @@ +1334,5 @@ > { > _this->list=NULL; > _this->current=NULL; > + PR_Free(_this); > + _this=NULL; This isn't going to do what you think it does.
Attachment #8679460 -
Flags: review?(martin.thomson) → review-
(Assignee) | ||
Comment 3•3 years ago
|
||
Created attachment 8680010 [details] [diff] [review] don't leak the iter uh, right. how about this?
Attachment #8679460 -
Attachment is obsolete: true
Attachment #8680010 -
Flags: review?(martin.thomson)
Comment 4•3 years ago
|
||
Comment on attachment 8680010 [details] [diff] [review] don't leak the iter Review of attachment 8680010 [details] [diff] [review]: ----------------------------------------------------------------- LGTM Deferring the commit.
Attachment #8680010 -
Flags: review?(martin.thomson) → review+
Comment 5•3 years ago
|
||
https://hg.mozilla.org/projects/nss/rev/fca456c49a53
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.22
You need to log in
before you can comment on or make changes to this bug.
Description
•