Closed
Bug 442012
Opened 17 years ago
Closed 16 years ago
Allocating more than 4GB using nsMemory::Alloc(size_t) succeeds but less memory is allocated
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: michal, Assigned: benjamin)
Details
(Keywords: fixed1.8.1.21, fixed1.9.0.6, fixed1.9.1, Whiteboard: [sg:low])
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
benjamin
:
review+
wtc
:
review+
dveditz
:
superreview+
beltzner
:
approval1.9.1+
dveditz
:
approval1.9.0.6+
dveditz
:
approval1.8.1.next+
asac
:
approval1.8.0.next+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
Type size_t is unsigned long int on 64-bit OS. So it is possible to ask for more than 4GB of memory using nsMemory::Alloc(size_t). But nsMemory::Alloc(size_t) calls NS_Alloc(PRSize) which calls PR_Malloc(PRUint32) where the most significant half of the parameter is thrown away. E.g. allocating 5GB of memory will succeed but only 1GB will be allocated.
Comment 1•16 years ago
|
||
Benjamin: assigning to you to figure out what approach we take. Change the nsMemory::Alloc() signature, or change the backend implementation to work correctly on 64-bit systems?
Surely NSPR has run into this problem, what do they do on 64-bit systems?
Assignee: nobody → benjamin
Comment 2•16 years ago
|
||
The NSPR bug for this issue is bug 445295.
You can change NS_Alloc(PRSize) to call malloc(size_t) instead
of PR_Malloc(PRUint32). Remember to call calloc and free instead
of PR_Calloc and PR_Free in NS_Realloc and NS_Free.
Comment 3•16 years ago
|
||
That should have been "Remember to call realloc and free instead
of PR_Realloc and PR_Free in NS_Realloc and NS_Free."
Assignee | ||
Comment 4•16 years ago
|
||
What size is PRSize on 64-bit systems?
Reporter | ||
Comment 5•16 years ago
|
||
(In reply to comment #4)
> What size is PRSize on 64-bit systems?
(gdb) whatis PRSize
type = size_t
(gdb) whatis size_t
type = long unsigned int
(gdb) p sizeof(PRSize)
$1 = 8
Assignee | ||
Comment 6•16 years ago
|
||
Let's just bail on allocations >2GB... I can't think of any reason they should be allowed.
Attachment #345539 -
Flags: superreview?(dveditz)
Attachment #345539 -
Flags: review?(dveditz)
Comment 7•16 years ago
|
||
Comment on attachment 345539 [details] [diff] [review]
Prevent allocations >2GB, rev. 1
Don't forget to patch NS_Realloc, too.
Attachment #345539 -
Flags: review+
Assignee | ||
Comment 8•16 years ago
|
||
Attachment #345539 -
Attachment is obsolete: true
Attachment #346730 -
Flags: superreview?(dveditz)
Attachment #346730 -
Flags: review+
Attachment #345539 -
Flags: superreview?(dveditz)
Attachment #345539 -
Flags: review?(dveditz)
Comment 9•16 years ago
|
||
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
r=wtc.
Attachment #346730 -
Flags: review+
Updated•16 years ago
|
Attachment #346730 -
Flags: superreview?(dveditz) → superreview+
Comment 10•16 years ago
|
||
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
sr=dveditz
Assignee | ||
Comment 11•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•16 years ago
|
||
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
Assuming the perf metrics aren't affected, this should be very safe.
Attachment #346730 -
Flags: approval1.9.1?
Attachment #346730 -
Flags: approval1.9.0.6?
Updated•16 years ago
|
Flags: wanted1.9.0.x+
Comment 13•16 years ago
|
||
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
a191=beltzner, didn't see any perf impact after landing.
Attachment #346730 -
Flags: approval1.9.1? → approval1.9.1+
Updated•16 years ago
|
Whiteboard: [needs 1.9.1 landing]
Updated•16 years ago
|
Whiteboard: [needs 1.9.1 landing] → [sg:low][needs 1.9.1 landing]
Assignee | ||
Comment 14•16 years ago
|
||
Fixed on 1.9.1, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5b26e181c6d5
Keywords: fixed1.9.1
Updated•16 years ago
|
Whiteboard: [sg:low][needs 1.9.1 landing] → [sg:low]
Comment 15•16 years ago
|
||
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
Approved for 1.9.0.6, a=dveditz for release-drivers.
Attachment #346730 -
Flags: approval1.9.0.6? → approval1.9.0.6+
Comment 17•16 years ago
|
||
Is there anything that really needs to be done to verified that this is fixed in 1.9.0.6?
Assignee | ||
Comment 18•16 years ago
|
||
I don't think you could verify it with a 32-bit OS. Since there's no testcase here, I think it can safely be left alone.
Comment 19•16 years ago
|
||
seems we want it on 1.8 too.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.next?
Flags: blocking1.8.0.next+
Updated•16 years ago
|
Attachment #346730 -
Flags: approval1.8.1.next?
Attachment #346730 -
Flags: approval1.8.0.next?
Updated•16 years ago
|
Flags: blocking1.8.1.next? → blocking1.8.1.next+
Comment 20•16 years ago
|
||
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
Approved for 1.8.1.21, a=dveditz for release-drivers.
Attachment #346730 -
Flags: approval1.8.1.next? → approval1.8.1.next+
Comment 21•16 years ago
|
||
fixed for 1.8.1.21
Checking in xpcom/base/nsMemoryImpl.cpp;
/cvsroot/mozilla/xpcom/base/nsMemoryImpl.cpp,v <-- nsMemoryImpl.cpp
new revision: 1.29.4.2; previous revision: 1.29.4.1
Keywords: fixed1.8.1.21
Comment 22•16 years ago
|
||
Updated•16 years ago
|
Group: core-security
Comment 23•16 years ago
|
||
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
a=asac for 1.8.0
Attachment #346730 -
Flags: approval1.8.0.next? → approval1.8.0.next+
You need to log in
before you can comment on or make changes to this bug.
Description
•