Closed Bug 115012 Opened 23 years ago Closed 23 years ago

32K local allocation in mpprime.c

Categories

(NSS :: Libraries, defect, P1)

Product:
NSS
Component:
Libraries
Version:
3.3.1
Platform:
x86
OS/2
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: alex, Assigned: nelson)

References

(
URL
)

Details

(Keywords: crash)

Attachments

(1 file)

Fix for problem
769 bytes, patch
wtc
: review+
Details | Diff | Splinter Review
From Bugzilla Helper: User-Agent: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:0.9.6) Gecko/20011127 BuildID: 2001112609 This bug is related to RSA key generation I think. When I fill and submit the form above mozilla displays a boy where it generates RSA key. During this process it craches. Here is pupup.log: === 12-12-2001 01:13:40 SYS3175 PID 11c5 TID 0001 Slot 0073 D:\INET\OS2WEB\MOZILLA.EXE c0000005 00b4fad9 P1=00000001 P2=0000001d P3=XXXXXXXX P4=XXXXXXXX EAX=02d000ff EBX=00b4fad4 ECX=00000000 EDX=00c80320 ESI=02d063b0 EDI=00000000 DS=0053 DSACC=f0f3 DSLIM=ffffffff ES=0053 ESACC=f0f3 ESLIM=ffffffff FS=150b FSACC=00f3 FSLIM=00000030 GS=0000 GSACC=**** GSLIM=******** CS:EIP=005b:00b4fad9 CSACC=f0df CSLIM=ffffffff SS:ESP=0053:0012f5f4 SSACC=f0f3 SSLIM=ffffffff EBP=0012f618 FLG=00010206 MOZRMI36.DLL ------------------------------------------------------------ 12-12-2001 07:24:47 SYS3175 PID 1931 TID 0004 Slot 0088 D:\INET\OS2WEB\MOZILLA.EXE c0000005 575f4758 P1=00000001 P2=575f4758 P3=XXXXXXXX P4=XXXXXXXX EAX=00000006 EBX=01010001 ECX=1d462a29 EDX=00b5ff04 ESI=01010101 EDI=01010101 DS=0053 DSACC=f0f3 DSLIM=ffffffff ES=0053 ESACC=f0f3 ESLIM=ffffffff FS=150b FSACC=00f3 FSLIM=00000030 GS=0000 GSACC=**** GSLIM=******** CS:EIP=005b:575f4758 CSACC=f0df CSLIM=ffffffff SS:ESP=0053:00b5fee8 SSACC=f0f3 SSLIM=ffffffff EBP=00010000 FLG=00010216 === Reproducible: Always Steps to Reproduce: 1.fill the form (check all boxes, fill something in textboxes) 2.submit 3.look at the key generation process Actual Results: crash Expected Results: proceed ;)
It generated fine for me with 2001-12-13-03 on Linux. Reporter, please try a newer build.
wfm using build 2001121312 on Linux.
Keywords: crash
->PSM, probably wfm.
Assignee: mstoltz → ssaux
Severity: critical → major
Component: Security: General → Client Library
Product: Browser → PSM
QA Contact: bsharma → junruh
Version: other → 2.2
worksforme. to qa.
Status: UNCONFIRMED → RESOLVED
Closed: 23 years ago
Priority: -- → P2
Resolution: --- → WORKSFORME
Target Milestone: --- → 2.2
No, I've tried the latest 2001121319 and the crash is still here: 12-15-2001 12:49:56 SYS3175 PID 004d TID 0001 Slot 0074 D:\INET\OS2WEB\MOZILLA.EXE c0000005 32353339 P1=00000001 P2=32353339 P3=XXXXXXXX P4=XXXXXXXX EAX=00e827d0 EBX=32353339 ECX=00000000 EDX=00e89e68 ESI=00e827d0 EDI=00000000 DS=0053 DSACC=f0f3 DSLIM=ffffffff ES=0053 ESACC=f0f3 ESLIM=ffffffff FS=150b FSACC=00f3 FSLIM=00000030 GS=0000 GSACC=**** GSLIM=******** CS:EIP=005b:32353339 CSACC=f0df CSLIM=ffffffff SS:ESP=0053:0012f834 SSACC=f0f3 SSLIM=ffffffff EBP=0012f858 FLG=00010206
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
I'll take for investigation. I can't connect to this website.. does it still exist?
Assignee: ssaux → mkaply
Yes, the site still works...
Os/2 barfs on a 32K character string pointer as a local variable in mpp_make_prime. Yes I said 32K. Who wrote this code? The fix for Os/2 is to take the macintosh path which should be the default path. Allocation 32K on the local heap is just bad. We need this fix bad. We can't generate prime numbers without it.
Assignee: mkaply → wtc
Status: UNCONFIRMED → NEW
Component: Client Library → Libraries
Ever confirmed: true
Product: PSM → NSS
QA Contact: junruh → sonja.mirtitsch
Summary: crash if I fill and submit the form above (when generating key) → Really bad code in mpprime.c - 32K local heap allocation
Target Milestone: 2.2 → 3.3.1
Version: 2.2 → 3.3.1
Attached patch Fix for problem — — Splinter Review
Have Os/2 go down the mac path - malloc 32K, don't get it from the local heap.
Wan-Teh, This bug means that key generation won't work at all in mozilla 0.9.7 on OS/2, making the new S/MIME support very hard to use (it works if you import keys, but you just can't create new keys ...). I just don't know if we have a proper mechanism to deliver this for 0.9.7 since the browser tree is supposed to be closed soon, and we just tagged an NSS 3.3.2 last week. I can check the fix into NSS 3.4 tip but that won't help until mozilla 0.9.8 .
Comment on attachment 61922 [details] [diff] [review] Fix for problem This patch is fine. r=wtc. Michael, if you want this fix in mozilla0.9.7, could you get the appropriate approval?
Attachment #61922 - Flags: review+
Assigned the bug to Nelson. Nelson, is it possible to make the sieve array static or allocate it from the heap for all platforms?
Assignee: wtc → nelsonb
Priority: P2 → P1
Target Milestone: 3.3.1 → 3.4
Allocating 32K from the stack is not "bad code", and works on all but two platforms we support. The code certainly could allocate this memory from the heap on all platforms.
Summary: Really bad code in mpprime.c - 32K local heap allocation → 32K local allocation in mpprime.c
I'll apply the submitted patch. In what branches should it be applied?
The default NSPR thread stack size is 64K on many platforms. On OS/2, it is only 32K (see _MD_DEFAULT_STACK_SIZE in mozilla/nsprpub/pr/include/md/_os2.h). This problem can also be fixed by increasing the default NSPR thread stack size to at least 64K on OS/2. This is a good thing to do because a 32K thread stack is pretty small.
I will try your change, but the problem we were seeing was an issue with the stack probe (an internal compiler issue) And as far as "bad code" vs. "good code", I would assert that putting a 32K variable in a function is at least not a very common thing to do. In addition, I notice that a memset is used to clean up the allocation on mac, but nothing is used to clean the 32K stack entity, so theoretically pieces of the private key are still on the stack.
I believe there's no need to clear the 32K sieve after the operation is done. I believe the prime found with the sieve cannot be reconstructed from the sieve. I'll check in the patch on the NSS_3_3_BRANCH (since that's where the NSS_CLIENT_TAG points, and no-one has requested anything else) and the trunk.
Status: NEW → ASSIGNED
I agree that 32K is big for a stack variable. There is performance advantage in using stack variables, especially in multithreaded programs, because that saves a trip to the heap.
Wan-Teh, It sounds like in this case the heap contention is only occurring during RSA keygen operation, right ? So it shouldn't critically affect performance for, say, SSL servers. Maybe CMS would be affected but I doubt it. But for the client malloc lock contention is probably not an issue.
Checked in revs 1.17 on trunk and 1.16.48.2 on NSS_3_3_BRANCH. Someone else will need to move the NSS_CLIENT_TAG if mozilla is to take this change.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
a=asa (on behalf of drivers) for checkin to 0.9.7
Keywords: mozilla0.9.7+
Who is going to move the NSS_CLIENT_TAG??
I don't know, who has the power to do that ?
WTC is absolutely correct. The core problem here was the OS/2 thread stack size. I've noted this in http://bugzilla.mozilla.org/show_bug.cgi?id=115149 I still believe in the change that was made, but I wouldn't worry about the CLIENT_TAG right now. The NSPR changes can take care of this.
So is this solved for 0.9.7?
I believe the answer is: not until someone with authority to move the NSS_CLIENT_TAG tag on mpprime.c does so.
I moved the NSS_CLIENT_TAG on mpprime.c. I haven't checked in the fix on the MOZILLA_0_9_7_BRANCH. Is it still okay to do that?
Blocks: 114455
I'd love to see it in 0.9.7 if it can go. I'll put it in my Os/2 build whether it is in the 0_9_7 branch or not.
No longer blocks: 114455
Verify, this code fix is in.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: