1273780 - ecperf coverity issues

Status: RESOLVED FIXED

(NSS :: Test, defect)

Description

[Franziskus Kiefer [:franziskus]]

Coverity found multiple issues in the re-enabled ecperf tests.

Comment 1

[Franziskus Kiefer [:franziskus]]

Attachment: ecperf-coverity.patch

fixing issues found by coverity in ecperf.
Most changes should be self explanatory.
The additional SEC_OID_UNKNOWN are necessary to make that map the size of ECCurve_pastLastCurve

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8754739 [details] [diff] [review]
ecperf-coverity.patch

Review of attachment 8754739 [details] [diff] [review]:
-----------------------------------------------------------------

::: cmd/ecperf/ecperf.c
@@ +210,5 @@
>                  rv = (*opfunc)(param1, param2, param3);
>              }
> +            if (rv != SECSuccess) {
> +                SECU_PrintError("Error:", op);
> +                return rv;

We'd leak |threadIDs| and |threadData| here, maybe introduce a "loser" or "cleanup" label or something?

@@ +404,5 @@
>      if (crv != CKR_OK) {
>          printf("Sign Failed CK_RV=0x%x\n", (int)crv);
>          return SECFailure;
>      }
> +    sig->len = (unsigned long)sigLen;

Maybe |sig->len = (CK_ULONG)sigLen| ?

::: lib/softoken/pkcs11c.c
@@ +2070,5 @@
>      switch (pMechanism->mechanism) {
>      case CKM_RC2_MAC_GENERAL:
> +	if (!pMechanism->pParameter) {
> +	    return CKR_ARGUMENTS_BAD;
> +	}

Hmmm is that right thing to return? Or rather CKR_MECHANISM_PARAM_INVALID?

Comment 3

[Franziskus Kiefer [:franziskus]]

landed as https://hg.mozilla.org/projects/nss/rev/83c8b17160fe