[CID 222162][CID 222161] overwrite_var: Overwriting subiter in subiter = Pk11Install_ListIter_new

RESOLVED FIXED in 3.22

Status

NSS
Libraries
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: fkiefer, Assigned: fkiefer)

Tracking

(Blocks: 1 bug, {coverity})

trunk
3.22
coverity

Firefox Tracking Flags

(firefox44 affected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
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 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-
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 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+
https://hg.mozilla.org/projects/nss/rev/fca456c49a53
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.22
You need to log in before you can comment on or make changes to this bug.