nspr provides incorrect declarations for big-endian ARM/Linux systems which breaks nss

RESOLVED FIXED in 4.8

Status

defect
--
major
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: mikpe, Assigned: wtc)

Tracking

other
ARM
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [4.7.5])

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.12) Gecko/20071126 Fedora/1.5.0.12-7.fc6 Firefox/1.5.0.12
Build Identifier: nspr-4.7.4

mozilla/nsprpub/pr/include/md/_linux.cfg unconditionally defines IS_LITTLE_ENDIAN on ARM/Linux systems, even though big-endian ARM/Linux systems are valid and not that uncommon.

The most obvious and serious consequence of this is that it causes NSS, which uses NSPR's installed include files, to perform byte-swapping operations when it should not, which breaks many of its crypto algorithms.

Reproducible: Always

Steps to Reproduce:
1. build nspr on a big-endian ARM system, e.g. armv5teb-unknown-linux-gnueabi
2. next build nss on that same system
3. run nss' test suite

Actual Results:  
most tests in nss' test suite fail


Expected Results:  
all tests should succeed

Fixing this is trivial, by using gcc's __ARMEB__ preprocessor macro to detect and handle big-endian configurations. I'll attach a working patch once the bug report is filed.
Reporter

Comment 1

10 years ago
The attached patch uses gcc's standard __ARMEB__ macro to test for big-endian configurations, and then makes _linux.cfg define IS_BIG_ENDIAN or IS_LITTLE_ENDIAN as appropriate. Tested on armv5teb (big-endian) and armv5tel (little-endian) machines by building nspr, then nss, and checking that nss' test suite passes.
Assignee

Updated

10 years ago
Attachment #375523 - Flags: review+
Assignee

Comment 2

10 years ago
Comment on attachment 375523 [details] [diff] [review]
patch to correct _linux.cfg on big-endian ARM/Linux

r=wtc.  Thanks for the patch.

I just did a search for __ARMEB__ and __ARMEL__ in our source tree.
We are handling ARM correctly only for NetBSD.  I also found that
We should be handling MIPS in the same way, which we already are
doing:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/include/md/_linux.cfg&rev=3.22&mark=472-480#470

I may check in a version of your patch that also checks __ARMEL__
explicitly.
Assignee

Comment 3

10 years ago
I edited Mikael Pettersson's patch a little bit and checked it in
on the NSPR trunk (NSPR 4.8).

Checking in _linux.cfg;
/cvsroot/mozilla/nsprpub/pr/include/md/_linux.cfg,v  <--  _linux.cfg
new revision: 3.23; previous revision: 3.22
done
Attachment #375523 - Attachment is obsolete: true
Assignee

Updated

10 years ago
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Hardware: Other → ARM
Resolution: --- → FIXED
Whiteboard: [consider for 4.7.5]
Target Milestone: --- → 4.8
Assignee

Comment 4

10 years ago
I backported the patch (attachment 375833 [details] [diff] [review]) to the NSPR_4_7_BRANCH
for NSPR 4.7.5.

Checking in _linux.cfg;
/cvsroot/mozilla/nsprpub/pr/include/md/_linux.cfg,v  <--  _linux.cfg
new revision: 3.22.4.1; previous revision: 3.22
done
Whiteboard: [consider for 4.7.5] → [4.7.5]
You need to log in before you can comment on or make changes to this bug.