free NULL cleanup

Start ensuring all OpenSSL "free" routines allow NULL, and remove any if check before calling them. This gets ASN1_OBJECT_free and ASN1_STRING_free. Reviewed-by: Matt Caswell <[email protected]>
handcraftedcode1 · Mar 24, 2015 · 0dfb939 · 0dfb939
1 parent 7c82e33
commit 0dfb939
Show file tree

Hide file tree

Showing 26 changed files with 61 additions and 71 deletions.
diff --git a/CHANGES b/CHANGES
@@ -71,6 +71,11 @@
         Remove all but one '#ifdef undef' which is to be looked at.
      [Rich Salz]
 
+  *) Clean up calling of xxx_free routines.
+        Just like free(), fix most of the xxx_free routines to accept
+        NULL.  Remove the non-null checks from callers.  Save much code.
+     [Rich Salz]
+
   *) Experimental support for a new, fast, unbiased prime candidate generator,
      bn_probable_prime_dh_coprime(). Not currently used by any prime generator.
      [Felix Laurie von Massenbach <[email protected]>]

diff --git a/apps/cms.c b/apps/cms.c
@@ -1152,8 +1152,7 @@ int MAIN(int argc, char **argv)
         OPENSSL_free(secret_keyid);
     if (pwri_tmp)
         OPENSSL_free(pwri_tmp);
-    if (econtent_type)
-        ASN1_OBJECT_free(econtent_type);
+    ASN1_OBJECT_free(econtent_type);
     if (rr)
         CMS_ReceiptRequest_free(rr);
     if (rr_to)

diff --git a/crypto/asn1/a_bitstr.c b/crypto/asn1/a_bitstr.c
@@ -177,7 +177,7 @@ ASN1_BIT_STRING *c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a,
     return (ret);
  err:
     ASN1err(ASN1_F_C2I_ASN1_BIT_STRING, i);
-    if ((ret != NULL) && ((a == NULL) || (*a != ret)))
+    if ((a == NULL) || (*a != ret))
         ASN1_BIT_STRING_free(ret);
     return (NULL);
 }

diff --git a/crypto/asn1/a_int.c b/crypto/asn1/a_int.c
@@ -265,7 +265,7 @@ ASN1_INTEGER *c2i_ASN1_INTEGER(ASN1_INTEGER **a, const unsigned char **pp,
     return (ret);
  err:
     ASN1err(ASN1_F_C2I_ASN1_INTEGER, i);
-    if ((ret != NULL) && ((a == NULL) || (*a != ret)))
+    if ((a == NULL) || (*a != ret))
         ASN1_INTEGER_free(ret);
     return (NULL);
 }
@@ -334,7 +334,7 @@ ASN1_INTEGER *d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp,
     return (ret);
  err:
     ASN1err(ASN1_F_D2I_ASN1_UINTEGER, i);
-    if ((ret != NULL) && ((a == NULL) || (*a != ret)))
+    if ((a == NULL) || (*a != ret))
         ASN1_INTEGER_free(ret);
     return (NULL);
 }

diff --git a/crypto/asn1/a_utctm.c b/crypto/asn1/a_utctm.c
@@ -193,11 +193,11 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t,
     int free_s = 0;
 
     if (s == NULL) {
-        free_s = 1;
         s = ASN1_UTCTIME_new();
+        if (s == NULL)
+            goto err;
+        free_s = 1;
     }
-    if (s == NULL)
-        goto err;
 
     ts = OPENSSL_gmtime(&t, &data);
     if (ts == NULL)
@@ -233,7 +233,7 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t,
 #endif
     return (s);
  err:
-    if (free_s && s)
+    if (free_s)
         ASN1_UTCTIME_free(s);
     return NULL;
 }

diff --git a/crypto/asn1/asn1_lib.c b/crypto/asn1/asn1_lib.c
@@ -429,7 +429,9 @@ void ASN1_STRING_free(ASN1_STRING *a)
 
 void ASN1_STRING_clear_free(ASN1_STRING *a)
 {
-    if (a && a->data && !(a->flags & ASN1_STRING_FLAG_NDEF))
+    if (a == NULL)
+        return;
+    if (a->data && !(a->flags & ASN1_STRING_FLAG_NDEF))
         OPENSSL_cleanse(a->data, a->length);
     ASN1_STRING_free(a);
 }

diff --git a/crypto/asn1/asn1_par.c b/crypto/asn1/asn1_par.c
@@ -276,10 +276,8 @@ static int asn1_parse2(BIO *bp, const unsigned char **pp, long length,
                         nl = 1;
                     }
                 }
-                if (os != NULL) {
-                    ASN1_OCTET_STRING_free(os);
-                    os = NULL;
-                }
+                ASN1_OCTET_STRING_free(os);
+                os = NULL;
             } else if (tag == V_ASN1_INTEGER) {
                 ASN1_INTEGER *bs;
                 int i;
@@ -356,10 +354,8 @@ static int asn1_parse2(BIO *bp, const unsigned char **pp, long length,
     }
     ret = 1;
  end:
-    if (o != NULL)
-        ASN1_OBJECT_free(o);
-    if (os != NULL)
-        ASN1_OCTET_STRING_free(os);
+    ASN1_OBJECT_free(o);
+    ASN1_OCTET_STRING_free(os);
     *pp = p;
     return (ret);
 }

diff --git a/crypto/asn1/evp_asn1.c b/crypto/asn1/evp_asn1.c
@@ -187,8 +187,7 @@ int ASN1_TYPE_get_int_octetstring(ASN1_TYPE *a, long *num,
  err:
         ASN1err(ASN1_F_ASN1_TYPE_GET_INT_OCTETSTRING, ASN1_R_DATA_IS_WRONG);
     }
-    if (os != NULL)
-        ASN1_OCTET_STRING_free(os);
+    ASN1_OCTET_STRING_free(os);
     if (ai != NULL)
         ASN1_INTEGER_free(ai);
     return (ret);

diff --git a/crypto/asn1/p5_pbe.c b/crypto/asn1/p5_pbe.c
@@ -118,8 +118,7 @@ int PKCS5_pbe_set0_algor(X509_ALGOR *algor, int alg, int iter,
  err:
     if (pbe != NULL)
         PBEPARAM_free(pbe);
-    if (pbe_str != NULL)
-        ASN1_STRING_free(pbe_str);
+    ASN1_STRING_free(pbe_str);
     return 0;
 }
 

diff --git a/crypto/asn1/tasn_fre.c b/crypto/asn1/tasn_fre.c
@@ -85,6 +85,7 @@ static void asn1_item_combine_free(ASN1_VALUE **pval, const ASN1_ITEM *it,
     const ASN1_AUX *aux = it->funcs;
     ASN1_aux_cb *asn1_cb;
     int i;
+
     if (!pval)
         return;
     if ((it->itype != ASN1_ITYPE_PRIMITIVE) && !*pval)
@@ -116,6 +117,7 @@ static void asn1_item_combine_free(ASN1_VALUE **pval, const ASN1_ITEM *it,
         i = asn1_get_choice_selector(pval, it);
         if ((i >= 0) && (i < it->tcount)) {
             ASN1_VALUE **pchval;
+
             tt = it->templates + i;
             pchval = asn1_get_field_ptr(pval, tt);
             ASN1_template_free(pchval, tt);
@@ -170,35 +172,41 @@ static void asn1_item_combine_free(ASN1_VALUE **pval, const ASN1_ITEM *it,
 
 void ASN1_template_free(ASN1_VALUE **pval, const ASN1_TEMPLATE *tt)
 {
-    int i;
     if (tt->flags & ASN1_TFLG_SK_MASK) {
         STACK_OF(ASN1_VALUE) *sk = (STACK_OF(ASN1_VALUE) *)*pval;
+        int i;
+
         for (i = 0; i < sk_ASN1_VALUE_num(sk); i++) {
-            ASN1_VALUE *vtmp;
-            vtmp = sk_ASN1_VALUE_value(sk, i);
+            ASN1_VALUE *vtmp = sk_ASN1_VALUE_value(sk, i);
+
             asn1_item_combine_free(&vtmp, ASN1_ITEM_ptr(tt->item), 0);
         }
         sk_ASN1_VALUE_free(sk);
         *pval = NULL;
-    } else
+    } else {
         asn1_item_combine_free(pval, ASN1_ITEM_ptr(tt->item),
                                tt->flags & ASN1_TFLG_COMBINE);
+    }
 }
 
 void ASN1_primitive_free(ASN1_VALUE **pval, const ASN1_ITEM *it)
 {
     int utype;
+
+    /* Special case: if 'it' is a primitive with a free_func, use that. */
     if (it) {
-        const ASN1_PRIMITIVE_FUNCS *pf;
-        pf = it->funcs;
+        const ASN1_PRIMITIVE_FUNCS *pf = it->funcs;
+
         if (pf && pf->prim_free) {
             pf->prim_free(pval, it);
             return;
         }
     }
-    /* Special case: if 'it' is NULL free contents of ASN1_TYPE */
+
+    /* Special case: if 'it' is NULL, free contents of ASN1_TYPE */
     if (!it) {
         ASN1_TYPE *typ = (ASN1_TYPE *)*pval;
+
         utype = typ->type;
         pval = &typ->value.asn1_value;
         if (!*pval)
@@ -235,8 +243,8 @@ void ASN1_primitive_free(ASN1_VALUE **pval, const ASN1_ITEM *it)
 
     default:
         ASN1_STRING_free((ASN1_STRING *)*pval);
-        *pval = NULL;
         break;
     }
-    *pval = NULL;
+    if (*pval)
+        *pval = NULL;
 }
diff --git a/crypto/asn1/x_algor.c b/crypto/asn1/x_algor.c
@@ -86,8 +86,7 @@ int X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, int ptype, void *pval)
             return 0;
     }
     if (alg) {
-        if (alg->algorithm)
-            ASN1_OBJECT_free(alg->algorithm);
+        ASN1_OBJECT_free(alg->algorithm);
         alg->algorithm = aobj;
     }
     if (ptype == 0)

diff --git a/crypto/asn1/x_pkey.c b/crypto/asn1/x_pkey.c
@@ -143,8 +143,7 @@ void X509_PKEY_free(X509_PKEY *x)
 
     if (x->enc_algor != NULL)
         X509_ALGOR_free(x->enc_algor);
-    if (x->enc_pkey != NULL)
-        ASN1_OCTET_STRING_free(x->enc_pkey);
+    ASN1_OCTET_STRING_free(x->enc_pkey);
     if (x->dec_pkey != NULL)
         EVP_PKEY_free(x->dec_pkey);
     if ((x->key_data != NULL) && (x->key_free))

diff --git a/crypto/asn1/x_x509a.c b/crypto/asn1/x_x509a.c
@@ -159,8 +159,7 @@ int X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj)
     if (!objtmp || sk_ASN1_OBJECT_push(aux->trust, objtmp))
         return 1;
  err:
-    if (objtmp)
-        ASN1_OBJECT_free(objtmp);
+    ASN1_OBJECT_free(objtmp);
     return 0;
 }
 

diff --git a/crypto/dh/dh_ameth.c b/crypto/dh/dh_ameth.c
@@ -191,8 +191,7 @@ static int dh_pub_encode(X509_PUBKEY *pk, const EVP_PKEY *pkey)
  err:
     if (penc)
         OPENSSL_free(penc);
-    if (str)
-        ASN1_STRING_free(str);
+    ASN1_STRING_free(str);
 
     return 0;
 }
@@ -297,8 +296,7 @@ static int dh_priv_encode(PKCS8_PRIV_KEY_INFO *p8, const EVP_PKEY *pkey)
  err:
     if (dp != NULL)
         OPENSSL_free(dp);
-    if (params != NULL)
-        ASN1_STRING_free(params);
+    ASN1_STRING_free(params);
     if (prkey != NULL)
         ASN1_STRING_clear_free(prkey);
     return 0;

diff --git a/crypto/dh/dh_pmeth.c b/crypto/dh/dh_pmeth.c
@@ -155,8 +155,7 @@ static void pkey_dh_cleanup(EVP_PKEY_CTX *ctx)
     if (dctx) {
         if (dctx->kdf_ukm)
             OPENSSL_free(dctx->kdf_ukm);
-        if (dctx->kdf_oid)
-            ASN1_OBJECT_free(dctx->kdf_oid);
+        ASN1_OBJECT_free(dctx->kdf_oid);
         OPENSSL_free(dctx);
     }
 }
@@ -245,8 +244,7 @@ static int pkey_dh_ctrl(EVP_PKEY_CTX *ctx, int type, int p1, void *p2)
         return dctx->kdf_ukmlen;
 
     case EVP_PKEY_CTRL_DH_KDF_OID:
-        if (dctx->kdf_oid)
-            ASN1_OBJECT_free(dctx->kdf_oid);
+        ASN1_OBJECT_free(dctx->kdf_oid);
         dctx->kdf_oid = p2;
         return 1;
 

diff --git a/crypto/dsa/dsa_ameth.c b/crypto/dsa/dsa_ameth.c
@@ -166,8 +166,7 @@ static int dsa_pub_encode(X509_PUBKEY *pk, const EVP_PKEY *pkey)
  err:
     if (penc)
         OPENSSL_free(penc);
-    if (str)
-        ASN1_STRING_free(str);
+    ASN1_STRING_free(str);
 
     return 0;
 }
@@ -328,8 +327,7 @@ static int dsa_priv_encode(PKCS8_PRIV_KEY_INFO *p8, const EVP_PKEY *pkey)
  err:
     if (dp != NULL)
         OPENSSL_free(dp);
-    if (params != NULL)
-        ASN1_STRING_free(params);
+    ASN1_STRING_free(params);
     if (prkey != NULL)
         ASN1_STRING_clear_free(prkey);
     return 0;

diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c
@@ -317,8 +317,7 @@ static int ec_asn1_group2fieldid(const EC_GROUP *group, X9_62_FIELDID *field)
         return 0;
 
     /* clear the old values (if necessary) */
-    if (field->fieldType != NULL)
-        ASN1_OBJECT_free(field->fieldType);
+    ASN1_OBJECT_free(field->fieldType);
     if (field->p.other != NULL)
         ASN1_TYPE_free(field->p.other);
 
@@ -654,7 +653,7 @@ ECPKPARAMETERS *ec_asn1_group2pkparameters(const EC_GROUP *group,
             return NULL;
         }
     } else {
-        if (ret->type == 0 && ret->value.named_curve)
+        if (ret->type == 0)
             ASN1_OBJECT_free(ret->value.named_curve);
         else if (ret->type == 1 && ret->value.parameters)
             ECPARAMETERS_free(ret->value.parameters);

diff --git a/crypto/ocsp/ocsp_lib.c b/crypto/ocsp/ocsp_lib.c
@@ -110,8 +110,7 @@ OCSP_CERTID *OCSP_cert_id_new(const EVP_MD *dgst,
         goto err;
 
     alg = cid->hashAlgorithm;
-    if (alg->algorithm != NULL)
-        ASN1_OBJECT_free(alg->algorithm);
+    ASN1_OBJECT_free(alg->algorithm);
     if ((nid = EVP_MD_type(dgst)) == NID_undef) {
         OCSPerr(OCSP_F_OCSP_CERT_ID_NEW, OCSP_R_UNKNOWN_NID);
         goto err;

diff --git a/crypto/ocsp/v3_ocsp.c b/crypto/ocsp/v3_ocsp.c
@@ -247,7 +247,7 @@ static void *d2i_ocsp_nonce(void *a, const unsigned char **pp, long length)
     return os;
 
  err:
-    if (os && (!pos || (*pos != os)))
+    if ((pos == NULL) || (*pos != os))
         ASN1_OCTET_STRING_free(os);
     OCSPerr(OCSP_F_D2I_OCSP_NONCE, ERR_R_MALLOC_FAILURE);
     return NULL;

diff --git a/crypto/rsa/rsa_ameth.c b/crypto/rsa/rsa_ameth.c
@@ -484,8 +484,7 @@ static int rsa_md_to_mgf1(X509_ALGOR **palg, const EVP_MD *mgf1md)
     X509_ALGOR_set0(*palg, OBJ_nid2obj(NID_mgf1), V_ASN1_SEQUENCE, stmp);
     stmp = NULL;
  err:
-    if (stmp)
-        ASN1_STRING_free(stmp);
+    ASN1_STRING_free(stmp);
     if (algtmp)
         X509_ALGOR_free(algtmp);
     if (*palg)
@@ -576,8 +575,7 @@ static ASN1_STRING *rsa_ctx_to_pss(EVP_PKEY_CTX *pkctx)
         RSA_PSS_PARAMS_free(pss);
     if (rv)
         return os;
-    if (os)
-        ASN1_STRING_free(os);
+    ASN1_STRING_free(os);
     return NULL;
 }
 
@@ -921,8 +919,7 @@ static int rsa_cms_encrypt(CMS_RecipientInfo *ri)
  err:
     if (oaep)
         RSA_OAEP_PARAMS_free(oaep);
-    if (os)
-        ASN1_STRING_free(os);
+    ASN1_STRING_free(os);
     return rv;
 }
 

diff --git a/crypto/rsa/rsa_saos.c b/crypto/rsa/rsa_saos.c
@@ -138,8 +138,7 @@ int RSA_verify_ASN1_OCTET_STRING(int dtype,
     } else
         ret = 1;
  err:
-    if (sig != NULL)
-        ASN1_OCTET_STRING_free(sig);
+    ASN1_OCTET_STRING_free(sig);
     if (s != NULL) {
         OPENSSL_cleanse(s, (unsigned int)siglen);
         OPENSSL_free(s);

diff --git a/crypto/x509v3/pcy_data.c b/crypto/x509v3/pcy_data.c
@@ -102,8 +102,7 @@ X509_POLICY_DATA *policy_data_new(POLICYINFO *policy,
     ret->expected_policy_set = sk_ASN1_OBJECT_new_null();
     if (!ret->expected_policy_set) {
         OPENSSL_free(ret);
-        if (id)
-            ASN1_OBJECT_free(id);
+        ASN1_OBJECT_free(id);
         return NULL;
     }
 

diff --git a/crypto/x509v3/v3_conf.c b/crypto/x509v3/v3_conf.c
@@ -212,8 +212,7 @@ static X509_EXTENSION *do_ext_i2d(const X509V3_EXT_METHOD *method,
     X509V3err(X509V3_F_DO_EXT_I2D, ERR_R_MALLOC_FAILURE);
     if (ext_der != NULL)
         OPENSSL_free(ext_der);
-    if (ext_oct != NULL)
-        ASN1_OCTET_STRING_free(ext_oct);
+    ASN1_OCTET_STRING_free(ext_oct);
     return NULL;
 
 }