Hi all,
While performing performance tests on a Sun Crypto Accelerator I Board (also known as CryptoSwift) on sparc-sun-solaris architecture, I noticed a race condition between the libswift.so-hardware-I/O and the alarm signal speed.c uses. The symptom was the openssl speed program terminating with the following openssl error message: 9728:error:26066072:engine routines:CSWIFT_MOD_EXP:request failed: hw_cswift.c:413:CryptoSwift error number is -10004 [or on the latest source snapshot: 2197:error:80068076:cswift engine code:cswift_mod_exp_crt:request failed:hw_cswift.c:680:CryptoSwift error number is -10004] [from cswift.h: -10004 is SW_ERR_NO_EXECUTE 'The Card failed to execute the command'] Obviously, libswift.so does not restart the ioctl call(s) that fails with EINTR when SIGALRM is received, as observed in a truss output. I don't know which libswift versions are affected by this problem; see below for version details on the one I tested. This problem does not only affect the 'openssl speed' code, but any code that uses the openssl engine library, cswift and signal handlers. There may be some other problematic signals besides SIGALRM (e.g. SIGCHLD, SIGUSR1, SIGUSR2, and probably more), so I decided to block (hold) all signals (but the non-blockable SIGKILL, etc.) during the libswift calls. [It is probably not necessary to protect the attach- parameters call and maybe neither the require-context and release- context, but it's safe, and the time penalty is probably minimal.] The tested libswift.so version is the following: libswift.so.5.2.2 (file date Jul 28 2000, 87196 bytes, md5sum fc43e2e71bdacf30337ce4f3f6252f6d, SUNWsecsn sun-solaris package version 1.0.0,REV=2000.08.09). Regards, Eric -- Eric Laroche <[EMAIL PROTECTED]>, AdNovum Informatik AG
diff -ur openssl-SNAP-20011126.orig/crypto/engine/hw_cswift.c openssl-SNAP-20011126/crypto/engine/hw_cswift.c --- openssl-SNAP-20011126.orig/crypto/engine/hw_cswift.c Tue Sep 25 22:02:20 2001 +++ openssl-SNAP-20011126/crypto/engine/hw_cswift.c Tue Nov 27 10:17:17 2001 @@ -255,6 +255,40 @@ static const char *engine_cswift_id = "cswift"; static const char *engine_cswift_name = "CryptoSwift hardware engine support"; +/* Signal blocking during libswift calls */ + +#if defined(unix) +#include <signal.h> +#endif + +static void crit_sect_on(void* sigctx, int size) { +#if defined(unix) + sigset_t s, t; + if (size < sizeof(s)) + return; + /* Block all signals. Note that typically, the sigprocmask + ** manual page states that it is not possible to block those + ** signals that cannot be ignored (e.g. SIGKILL and SIGSTOP) + ** but that this restriction is silently imposed by the system. + */ + sigfillset(&t); + sigprocmask(SIG_SETMASK, &t, &s); + /* [To have proper data alignment, don't use 'sigctx' directly.] */ + memcpy(sigctx, &s, sizeof(s)); +#endif +} + +static void crit_sect_off(void* sigctx, int size) { +#if defined(unix) + sigset_t s; + if (size < sizeof(s)) + return; + /* [To have proper data alignment, don't use 'sigctx' directly.] */ + memcpy(&s, sigctx, sizeof(s)); + sigprocmask(SIG_SETMASK, &s, NULL); +#endif +} + /* This internal function is used by ENGINE_cswift() and possibly by the * "dynamic" ENGINE support too */ static int bind_helper(ENGINE *e) @@ -365,8 +399,11 @@ static int get_context(SW_CONTEXT_HANDLE *hac) { SW_STATUS status; + char sigctx[32]; + crit_sect_on(sigctx, sizeof(sigctx)); status = p_CSwift_AcquireAccContext(hac); + crit_sect_off(sigctx, sizeof(sigctx)); if(status != SW_OK) return 0; return 1; @@ -375,7 +412,11 @@ /* similarly to release one. */ static void release_context(SW_CONTEXT_HANDLE hac) { + char sigctx[32]; + + crit_sect_on(sigctx, sizeof(sigctx)); p_CSwift_ReleaseAccContext(hac); + crit_sect_off(sigctx, sizeof(sigctx)); } /* Destructor (complements the "ENGINE_cswift()" constructor) */ @@ -506,6 +547,7 @@ SW_PARAM sw_param; SW_CONTEXT_HANDLE hac; int to_return, acquired; + char sigctx[32]; modulus = exponent = argument = result = NULL; to_return = 0; /* expect failure */ @@ -541,8 +583,10 @@ sw_param.up.exp.exponent.nbytes = BN_bn2bin(p, (unsigned char *)exponent->d); sw_param.up.exp.exponent.value = (unsigned char *)exponent->d; + crit_sect_on(sigctx, sizeof(sigctx)); /* Attach the key params */ sw_status = p_CSwift_AttachKeyParam(hac, &sw_param); + crit_sect_off(sigctx, sizeof(sigctx)); switch(sw_status) { case SW_OK: @@ -565,9 +609,11 @@ res.nbytes = BN_num_bytes(m); memset(result->d, 0, res.nbytes); res.value = (unsigned char *)result->d; + crit_sect_on(sigctx, sizeof(sigctx)); /* Perform the operation */ - if((sw_status = p_CSwift_SimpleRequest(hac, SW_CMD_MODEXP, &arg, 1, - &res, 1)) != SW_OK) + sw_status = p_CSwift_SimpleRequest(hac, SW_CMD_MODEXP, &arg, 1, &res, 1); + crit_sect_off(sigctx, sizeof(sigctx)); + if(sw_status != SW_OK) { char tmpbuf[20]; CSWIFTerr(CSWIFT_F_CSWIFT_MOD_EXP,CSWIFT_R_REQUEST_FAILED); @@ -603,6 +649,7 @@ BIGNUM *result = NULL; int to_return = 0; /* expect failure */ int acquired = 0; + char sigctx[32]; if(!get_context(&hac)) { @@ -648,8 +695,10 @@ sw_param.up.crt.iqmp.nbytes = BN_bn2bin(iqmp, (unsigned char *)rsa_iqmp->d); sw_param.up.crt.iqmp.value = (unsigned char *)rsa_iqmp->d; + crit_sect_on(sigctx, sizeof(sigctx)); /* Attach the key params */ sw_status = p_CSwift_AttachKeyParam(hac, &sw_param); + crit_sect_off(sigctx, sizeof(sigctx)); switch(sw_status) { case SW_OK: @@ -672,9 +721,11 @@ res.nbytes = 2 * BN_num_bytes(p); memset(result->d, 0, res.nbytes); res.value = (unsigned char *)result->d; + crit_sect_on(sigctx, sizeof(sigctx)); /* Perform the operation */ - if((sw_status = p_CSwift_SimpleRequest(hac, SW_CMD_MODEXP_CRT, &arg, 1, - &res, 1)) != SW_OK) + sw_status = p_CSwift_SimpleRequest(hac, SW_CMD_MODEXP_CRT, &arg, 1, &res, 1); + crit_sect_off(sigctx, sizeof(sigctx)); + if(sw_status != SW_OK) { char tmpbuf[20]; CSWIFTerr(CSWIFT_F_CSWIFT_MOD_EXP_CRT,CSWIFT_R_REQUEST_FAILED); @@ -737,6 +788,7 @@ BIGNUM *result = NULL; DSA_SIG *to_return = NULL; int acquired = 0; + char sigctx[32]; if((ctx = BN_CTX_new()) == NULL) goto err; @@ -780,8 +832,10 @@ sw_param.up.dsa.key.nbytes = BN_bn2bin(dsa->priv_key, (unsigned char *)dsa_key->d); sw_param.up.dsa.key.value = (unsigned char *)dsa_key->d; + crit_sect_on(sigctx, sizeof(sigctx)); /* Attach the key params */ sw_status = p_CSwift_AttachKeyParam(hac, &sw_param); + crit_sect_off(sigctx, sizeof(sigctx)); switch(sw_status) { case SW_OK: @@ -804,9 +858,11 @@ res.nbytes = BN_num_bytes(dsa->p); memset(result->d, 0, res.nbytes); res.value = (unsigned char *)result->d; + crit_sect_on(sigctx, sizeof(sigctx)); /* Perform the operation */ sw_status = p_CSwift_SimpleRequest(hac, SW_CMD_DSS_SIGN, &arg, 1, &res, 1); + crit_sect_off(sigctx, sizeof(sigctx)); if(sw_status != SW_OK) { char tmpbuf[20]; @@ -849,6 +905,7 @@ BIGNUM *argument = NULL; int to_return = -1; int acquired = 0; + char sigctx[32]; if((ctx = BN_CTX_new()) == NULL) goto err; @@ -892,8 +949,10 @@ sw_param.up.dsa.key.nbytes = BN_bn2bin(dsa->pub_key, (unsigned char *)dsa_key->d); sw_param.up.dsa.key.value = (unsigned char *)dsa_key->d; + crit_sect_on(sigctx, sizeof(sigctx)); /* Attach the key params */ sw_status = p_CSwift_AttachKeyParam(hac, &sw_param); + crit_sect_off(sigctx, sizeof(sigctx)); switch(sw_status) { case SW_OK: @@ -920,9 +979,11 @@ BN_bn2bin(sig->s, arg[1].value + 40 - BN_num_bytes(sig->s)); res.nbytes = 4; /* unsigned long */ res.value = (unsigned char *)(&sig_result); + crit_sect_on(sigctx, sizeof(sigctx)); /* Perform the operation */ sw_status = p_CSwift_SimpleRequest(hac, SW_CMD_DSS_VERIFY, arg, 2, &res, 1); + crit_sect_off(sigctx, sizeof(sigctx)); if(sw_status != SW_OK) { char tmpbuf[20];