[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openssl-dev
Subject:    Re: [PATCH] cswift engine code and signals (e.g. SIGALRM in speed.c)
From:       Tom Anderson <Tom.Anderson () cp ! net>
Date:       2001-11-27 17:16:43
[Download RAW message or body]


I didn't see which version of Solaris you were running, but be
aware that in 2.8, prior to KJP 12, there are some known kernel
and libaio issues that can result in the signal mask being 
changed out from under a process.

Eric Laroche wrote:
> 
> 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 <eric.laroche@adnovum.ch>, 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];

-- 
Tom Anderson			| Ex ignorantia ad sapientiam
Director, Testing and Tools	| e tenbris ad lucem!
Tom.Anderson@cp.net		| - someone famous i'm sure!
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@openssl.org
Automated List Manager                           majordomo@openssl.org


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic