Last Comment Bug 473147 - pk11mode tests fails on AIX when using shareable DBs.
: pk11mode tests fails on AIX when using shareable DBs.
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: trunk
: Other AIX
: P2 normal (vote)
: 3.12.3
Assigned To: glen beasley
:
:
Mentors:
http://mxr.mozilla.org/security/searc...
Depends on: 485238
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-12 04:54 PST by Slavomir Katuscak
Modified: 2009-04-06 12:53 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
fix for aix and verbose output (22.96 KB, patch)
2009-03-23 15:29 PDT, glen beasley
nelson: review-
Details | Diff | Splinter Review
fix for aix, pk11mode verbose output, and pk11util/pk11mode both using pk11table.c (155.11 KB, patch)
2009-03-25 13:29 PDT, glen beasley
no flags Details | Diff | Splinter Review
have pk11mode and pk11util share pk11table.c plus cleanup (28.48 KB, patch)
2009-03-26 19:37 PDT, glen beasley
rrelyea: review+
Details | Diff | Splinter Review
file pk11table.c and pk11table.h (69.36 KB, patch)
2009-03-26 19:46 PDT, glen beasley
no flags Details | Diff | Splinter Review
patch checked in (32.70 KB, patch)
2009-03-30 16:42 PDT, glen beasley
no flags Details | Diff | Splinter Review
fix regression re-apply patch 286123 from bug 401071 (833 bytes, patch)
2009-03-31 08:01 PDT, glen beasley
no flags Details | Diff | Splinter Review

Description Slavomir Katuscak 2009-01-12 04:54:08 PST
fips.sh: Run PK11MODE in FIPSMODE  -----------------
pk11mode -d ../fips -p fips- -f ../tests.fipspw.282782
Loaded FC_GetFunctionList for FIPS MODE; slotID 0 
Loaded FC_GetFunctionList for FIPS MODE; slotID 0 

FIPS MODE PKM_Error: [memory allocation of 0 bytes failed]

FIPS MODE PKM_Error: PKM_FindAllObjects failed with 0x00000012, CKR_ATTRIBUTE_TYPE_INVALID          
Loaded FC_GetFunctionList for FIPS MODE; slotID 0 
**** Total number of TESTS ran in FIPS MODE is 83. ****
fips.sh: #7538: Run PK11MODE in FIPS mode (pk11mode) . - FAILED
fips.sh: Run PK11MODE in Non FIPSMODE  -----------------
pk11mode -d ../fips -p nonfips- -f ../tests.fipspw.282782 -n
loaded C_GetFunctionList for NON FIPS MODE; slotID 1 
loaded C_GetFunctionList for NON FIPS MODE; slotID 1 
NON FIPS MODE PKM_Error: [memory allocation of 0 bytes failed]
NON FIPS MODE PKM_Error: PKM_FindAllObjects failed with 0x00000012, CKR_ATTRIBUTE_TYPE_INVALID          
loaded C_GetFunctionList for NON FIPS MODE; slotID 1 
**** Total number of TESTS ran in NON FIPS MODE is 81. ****
fips.sh: #7539: Run PK11MODE in Non FIPS mode (pk11mode -n) . - FAILED

This problem is there probably for longer time, but we started to test this combination on AIX only now.
Comment 1 Slavomir Katuscak 2009-01-12 04:56:26 PST
Those tests fails only with shareable DB format (export NSS_DEFAULT_DB_TYPE=sql) and probably only on AIX.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2009-01-12 12:24:42 PST
Reassigning to Glen.

Clearly, the calls to calloc are not preceded by a check that the amount 
to be allocated is non-zero.  That should be fixed, and a better diagnostic
message should be output in that case.

http://mxr.mozilla.org/security/search?string=memory%20allocation%20of
shows that the same exact error string is used in numerous places in the
same program, making it impossible to determine from the program's output
which of the various places experienced the failure.  That should be fixed
also at the same time as this bug is fixed.
Comment 3 glen beasley 2009-03-23 15:29:16 PDT
Created attachment 368973 [details] [diff] [review]
fix for aix and verbose output

the AIX build is failing in PKM_FindAllObjects when the DB type is sql because PKM_FindAllObjects is finding public key objects. Then trying to show all attributes of the public key object. A check is needed to ensure that the attribute has a valueLen greater than 0.

in verbose mode PKM_FindAllobjects had several formating issues, so this 
patch cleans up the verbose output. pk11mode was missing some attribute constants and rather than update pk11mode constant table I decided it was best to use pk11util's constant table in pk11table.c . I then had pk11mode get the mechanism and results from the same Constant table.

I was debating about how best to have pk11util and pk11mode share pk11table.c so I would like advice.
Comment 4 Nelson Bolyard (seldom reads bugmail) 2009-03-23 15:46:56 PDT
Comment on attachment 368973 [details] [diff] [review]
fix for aix and verbose output

Move pk11table.c to nss/cmd/lib, build it there, not in the make files for the commands that use it.  
If there's a header file for it, that must move too.
Use const whereever possible and appropriate in that table file.
Comment 5 glen beasley 2009-03-25 13:29:30 PDT
Created attachment 369349 [details] [diff] [review]
fix for aix, pk11mode verbose output, and pk11util/pk11mode both using pk11table.c

fix for aix, pk11mode verbose output, and pk11util/pk11mode both using pk11table.c

note: I did a cvs add on cmd/lib/pk11table.c and cmd/lib/pk11util.h 
only to show the files in this patch. I opened bug 485238 to move the files. Once files are moved I will checkout new tree then apply the patch, then checkin when patch is r+.

the AIX build is failing in PKM_FindAllObjects when the DB type is sql because
PKM_FindAllObjects is finding public key objects. Then trying to show all
attributes of the public key object. A check is needed to ensure that the
attribute has a valueLen greater than 0.

in verbose mode PKM_FindAllobjects had several formating issues, so this 
patch cleans up the verbose output. pk11mode was missing some attribute
constants and rather than update pk11mode constant table I decided it was best
to use pk11util's constant table in pk11table.c . I then had pk11mode get the
mechanism and results from the same Constant table.

remove warnings in pk11mode.
Comment 6 glen beasley 2009-03-26 19:37:13 PDT
Created attachment 369614 [details] [diff] [review]
have pk11mode and pk11util share pk11table.c plus cleanup


note: I did a cvs add on cmd/lib/pk11table.c and cmd/lib/pk11table.h 
only to show the files in this patch. I opened bug 485238 to move the files.
Once files are moved I will checkout new tree then apply the patch, then
checkin when patch is r+.

fix for aix, pk11mode verbose output, and pk11util/pk11mode both using
pk11table.c


the AIX build is failing in PKM_FindAllObjects when the DB type is sql because
PKM_FindAllObjects is finding public key objects. Then trying to show all
attributes of the public key object. A check is needed to ensure that the
attribute has a valueLen greater than 0.

in verbose mode PKM_FindAllobjects had several formating issues, so this 
patch cleans up the verbose output. pk11mode was missing some attribute
constants and rather than update pk11mode constant table I decided it was best
to use pk11util's constant table in pk11table.c . I then had pk11mode get the
mechanism and results from the same Constant table.

remove warnings in pk11mode.c and pk11util.c
Comment 7 glen beasley 2009-03-26 19:46:18 PDT
Created attachment 369615 [details] [diff] [review]
file pk11table.c and pk11table.h

new files pk11table.c and pk11table.h. I will do a proper diff if and when bug 485238 r+ and checked in to move pk11table.c and pk11table.h
Comment 8 Robert Relyea 2009-03-27 16:57:23 PDT
Comment on attachment 369614 [details] [diff] [review]
have pk11mode and pk11util share pk11table.c plus cleanup

r+ rrelyea, though I suggest the following change.

around line 2115 replace:


+                int column = 40;
+                printf("%s", mechName);
+                if (strlen(mechName) < column) {
+                    int j;
+                    for (j=0; j < (column - strlen(mechName)); j++) 
+                        printf(" "); 
+                }

with
+               printf("%-40s",mechName);


bob
Comment 9 glen beasley 2009-03-30 16:42:40 PDT
Created attachment 370098 [details] [diff] [review]
patch checked in



/cvsroot/mozilla/security/nss/cmd/lib/manifest.mn,v  <--  manifest.mn
new revision: 1.12; previous revision: 1.11
done
Checking in lib/pk11table.c;
/cvsroot/mozilla/security/nss/cmd/lib/pk11table.c,v  <--  pk11table.c
new revision: 1.11; previous revision: 1.10
done
Checking in lib/pk11table.h;
/cvsroot/mozilla/security/nss/cmd/lib/pk11table.h,v  <--  pk11table.h
new revision: 1.7; previous revision: 1.6
done
Checking in pk11mode/Makefile;
/cvsroot/mozilla/security/nss/cmd/pk11mode/Makefile,v  <--  Makefile
new revision: 1.4; previous revision: 1.3
done
Checking in pk11mode/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/pk11mode/manifest.mn,v  <--  manifest.mn
new revision: 1.5; previous revision: 1.4
done
Checking in pk11mode/pk11mode.c;
/cvsroot/mozilla/security/nss/cmd/pk11mode/pk11mode.c,v  <--  pk11mode.c
new revision: 1.24; previous revision: 1.23
done
Checking in pk11util/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/pk11util/manifest.mn,v  <--  manifest.mn
new revision: 1.3; previous revision: 1.2
done
Removing pk11util/pk11table.c;
/cvsroot/mozilla/security/nss/cmd/pk11util/pk11table.c,v  <--  pk11table.c
new revision: delete; previous revision: 1.10
done
Checking in pk11util/pk11util.c;
/cvsroot/mozilla/security/nss/cmd/pk11util/pk11util.c,v  <--  pk11util.c
new revision: 1.13; previous revision: 1.12
done
Removing pk11util/pk11util.h;
/cvsroot/mozilla/security/nss/cmd/pk11util/pk11util.h,v  <--  pk11util.h
new revision: delete; previous revision: 1.6
done
Comment 10 Julien Pierre 2009-03-30 23:01:08 PDT
This patch caused a regression on Win64. pk11mode now crashes with the following stack :

 	softokn3.dll!sftk_argStrip(char * c=0x0263c05000000001)  Line 98 + 0x5 bytes	C
 	softokn3.dll!sftk_parseParameters(char * param=0x0263c05000000001, sftk_parametersStr * parsed=0x000000000012fc20, int isFIPS=1)  Line 423 + 0xd bytes	C
>	softokn3.dll!nsc_CommonInitialize(void * pReserved=0x000000000012fe80, int isFIPS=1)  Line 2585 + 0x1b bytes	C
 	softokn3.dll!FC_Initialize(void * pReserved=0x000000000012fe80)  Line 508 + 0x12 bytes	C
 	pk11mode.exe!main(int argc=7, char * * argv=0x00000000026392d0)  Line 504 + 0x13 bytes	C
 	pk11mode.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	kernel32.dll!00000000775f495d() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
 	ntdll.dll!00000000777f8791()
Comment 11 Julien Pierre 2009-03-30 23:05:56 PDT
The cause is your removal of

#include "pkcs11p.h"
and
#include "pkcs11u.h"

around the definition CK_C_INITIALIZE_ARGS_NSS .

These include files define structure packing, and are required, otherwise the definition of the structure does not match between pk11mode and softoken.
Comment 12 Julien Pierre 2009-03-30 23:11:37 PDT
See bug 401071 for more information . Re-applying attachment 286123 [details] [diff] [review] should fix the problem.
Comment 13 glen beasley 2009-03-31 08:01:31 PDT
Created attachment 370196 [details] [diff] [review]
fix regression re-apply patch 286123 from bug 401071

thanks julien! I re-applied patch for win64.
Comment 14 Nelson Bolyard (seldom reads bugmail) 2009-04-06 12:50:24 PDT
What is the status of this bug?
still unfixed?
fixed in 3.12.3?  
or ??
Comment 15 glen beasley 2009-04-06 12:53:31 PDT
sorry this patch was checked in for 3.12.3. closing.

Note You need to log in before you can comment on or make changes to this bug.