168722 - core dump in SSL3 RSA WITH AES 128 CBC SHA tests

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Sonja Mirtitsch]

Tinderbox and QA failures since 9/13 evening. Time of breakage is when Julien's
checkin's "Fix compiler warnings" were made.

charm.3 SSL3 RSA WITH AES 128 CBC SHA Failed
charm.3 10 Fatal - selfserv process not detectable Failed
compaqtor.1 SSL3 RSA WITH AES 128 CBC SHA Failed
compaqtor.1 10 Fatal - selfserv process not detectable Failed
compaqtor.1 SSL3 RSA WITH AES 128 CBC SHA Failed
compaqtor.1 10 Fatal - selfserv process not detectable Failed
compaqtor.2 SSL3 RSA WITH AES 128 CBC SHA Failed
compaqtor.2 10 Fatal - selfserv process not detectable Failed
compaqtor.2 SSL3 RSA WITH AES 128 CBC SHA Failed
compaqtor.2 10 Fatal - selfserv process not detectable Failed



ssl.sh: running TLS RSA WITH NULL SHA ----------------------------
tstclnt -p 8443 -h galileo -c z  \
        -f -d ../client <
/share/builds/mccrel3/nss/nsstip/builds/20020914.1/booboo_Solaris8/mozilla/security/nss/tests/ssl/sslreq.txt
tstclnt: unable to connect: Connection refused by peer.
7160 Segmentation Fault - core dumped
/share/builds/mccrel3/nss/nsstip/builds/20020914.1/booboo_Solaris8/mozilla/security/nss/tests/all.sh:
kill: no such process

Comment 1

[Bishakha Banerjee]

I've made the core files on today's build QA results under the following dirs
(at Netscape) readable by all

haddock:/share/builds/sbsrel2/nss/nsstip/builds/20020916.1 11% find . -name core
 -print
./spd04_Solaris8/mozilla/tests_results/security/diffie.1/ext_client/core
./spd04_Solaris8/mozilla/tests_results/security/diffie.1/client/core
./spd04_Solaris8/mozilla/tests_results/security/diffie.1/ronlydir/core
./spd04_Solaris8/mozilla/tests_results/security/diffie.2/ext_client/core
./spd04_Solaris8/mozilla/tests_results/security/diffie.2/client/core
./spd04_Solaris8/mozilla/tests_results/security/diffie.2/ronlydir/core
./spd04_Solaris8/mozilla/tests_results/security/spd04.3/ext_client/core
./spd04_Solaris8/mozilla/tests_results/security/spd04.3/client/core
./spd04_Solaris8/mozilla/tests_results/security/spd04.3/ronlydir/core
./spd04_Solaris8/mozilla/tests_results/security/spd04.4/ext_client/core
./spd04_Solaris8/mozilla/tests_results/security/spd04.4/client/core
./spd04_Solaris8/mozilla/tests_results/security/spd04.4/ronlydir/core

Comment 2

[Bishakha Banerjee]

sorry, just checked Tinderbox and see this was fixed by Julien after yesterday
night's builds ran. So today morning's QA report does not reflect the fix.

Should be picked up by tonight's build and QA

Comment 3

[Wan-Teh Chang]

Julien backed out his checkin, so this has been fixed.

Ian checked in the same patch the previous weekend,
and I backed it out to fix the QA failures (in all
64-bit builds).  The intention of the patch is correct,
but the use of unsigned types makes some expressions
unsigned.  So the key is to find out which of the
affected expressions should not be unsigned.

Comment 4

[Julien Pierre]

Yes. FYI, I created the patch on friday 9/6. But I wasn't sure about it, so I
sent it to Ian for review and asked him to check it in. He replied by e-mail
that it was OK.

Then on friday 9/13, as I was cleaning up my NSS build tree, I noticed the patch
wasn't checked in (or so I thought, according to my diff), so I checked it in. I
didn't realize that it had been checked in by Ian and backed out already by
Wan-Teh because of this problem.

Comment 5

[Wan-Teh Chang]

Attachment: Proposed patch

The problem with Julien's patch is that 'j - blocksize' in
	for (j=0; j<blocksize; ++j)
	    out[j] ^= in[j - blocksize];
becomes an unsigned int expression but should be negative.
On 64-bit platforms, 'j - blocksize' needs to be promoted
to a 64-bit pointer (difference), so if it is unsigned,
the result of the promotion is a positive pointer difference,
which is wrong.

There are two ways to fix this.

1. Replace 'in[j - blocksize]' by '*(in + j - blocksize)'.
2. Replace 'in[j - blocksize]' by 'in[(int)(j - blocksize)]'.

This patch uses the second fix.

Comment 6

[Ian McGreer]

Comment on attachment 99471 [details] [diff] [review]
Proposed patch


just to make it official

Comment 7

[Wan-Teh Chang]

Comment on attachment 99471 [details] [diff] [review]
Proposed patch

This patch was checked in last night.