Closed Bug 337014 Opened 15 years ago Closed 15 years ago

Coverity OOM crash [@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen][@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen] Dereferencing possibly NULL "varena"

Categories

(NSS :: Libraries, defect, P2)

3.11
All
Linux
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.11.2

People

(Reporter: timeless, Assigned: alvolkov.bgs)

References

()

Details

(Keywords: coverity, crash, Whiteboard: CID 528)

Crash Data

Attachments

(1 file)

Event returned_null: Function "PORT_NewArena" returned NULL value (checked 219 out of 231 times) [model]
Hardware: PC → All
Target Milestone: --- → 3.11.2
Priority: -- → P2
Assignee: nobody → alexei.volkov.bugs
Attachment #222753 - Flags: review?(nelson)
Comment on attachment 222753 [details] [diff] [review]
check arena value after creation

>     varena = PORT_NewArena(60);
>+    if (!varena) {
>+	PORT_SetError(SEC_ERROR_NO_MEMORY);
>+	goto loser;
>+    }        
>+

When I first reviewed this patch, I thought to myself: "it should be unecessary
to set SEC_ERROR_NO_MEMORY after an allocation failure, because all PORT_ 
allocation functions should set that before they return."  Then I examined 
PORT_NewArena and discovered to my chagrin that it does not.

So, it is correct to tset the error code here, as this patch does.
However, I think the "right fix" is not to add a PORT_SetError call after 
every call to PORT_NewArena, but rather is to fix PORT_NewArena.
I have filed a separate bug about that.

So, I give this patch r+ now, with the knowledge that once we fix PORT_NewArena,
coverity will begin to report dead code complaints, about the error code 
being set twice in a row to the same value.
Attachment #222753 - Flags: review?(nelson) → review+
Integrated patch does not invoke PORT_SetError in case when varena is null (see comment #2).
 
trunk:
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11pqg.c,v  <--  pk11pqg.c
new revision: 1.12; previous revision: 1.11

3.11 branch:
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11pqg.c,v  <--  pk11pqg.c
new revision: 1.11.2.1; previous revision: 1.11
fwiw, coverity at present can't complain about double calls. although that isn't to say that a future version won't :).
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
CID 528.
Whiteboard: CID 528
Summary: OOM crash [@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen][@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen] Dereferencing possibly NULL "varena" → Coverity OOM crash [@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen][@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen] Dereferencing possibly NULL "varena"
Crash Signature: [@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen] [@ PORT_ArenaAlloc - PK11_PQG_ParamGenSeedLen]
You need to log in before you can comment on or make changes to this bug.