From a900668f4a0fcd0a05cfc5e3957ea4a53e5d4ea0 Mon Sep 17 00:00:00 2001 From: jhb Date: Thu, 25 Jun 2020 20:17:34 +0000 Subject: [PATCH] Use zfree() instead of explicit_bzero() and free(). In addition to reducing lines of code, this also ensures that the full allocation is always zeroed avoiding possible bugs with incorrect lengths passed to explicit_bzero(). Suggested by: cem Reviewed by: cem, delphij Approved by: csprng (cem) Sponsored by: Chelsio Communications Differential Revision: https://reviews.freebsd.org/D25435 --- sys/crypto/aesni/aesni.c | 18 ++++++------------ sys/crypto/via/padlock_cipher.c | 3 +-- sys/dev/cxgbe/crypto/t4_kern_tls.c | 3 +-- sys/dev/random/randomdev.c | 3 +-- sys/geom/eli/g_eli.c | 5 +---- sys/geom/eli/g_eli_ctl.c | 9 +++------ sys/geom/eli/g_eli_key_cache.c | 6 ++---- sys/geom/geom_dev.c | 5 +---- sys/kern/kern_environment.c | 3 +-- sys/kern/kern_shutdown.c | 21 +++++---------------- sys/kern/uipc_ktls.c | 7 ++----- sys/netinet/netdump/netdump_client.c | 6 +----- sys/opencrypto/crypto.c | 3 +-- sys/opencrypto/cryptosoft.c | 17 ++--------------- sys/opencrypto/ktls_ocf.c | 3 +-- 15 files changed, 29 insertions(+), 83 deletions(-) diff --git a/sys/crypto/aesni/aesni.c b/sys/crypto/aesni/aesni.c index 376487edf39a..42973b50718a 100644 --- a/sys/crypto/aesni/aesni.c +++ b/sys/crypto/aesni/aesni.c @@ -808,18 +808,12 @@ aesni_cipher_crypt(struct aesni_session *ses, struct cryptop *crp, crp->crp_payload_length, outbuf); out: - if (allocated) { - explicit_bzero(buf, crp->crp_payload_length); - free(buf, M_AESNI); - } - if (authallocated) { - explicit_bzero(authbuf, crp->crp_aad_length); - free(authbuf, M_AESNI); - } - if (outallocated) { - explicit_bzero(outbuf, crp->crp_payload_length); - free(outbuf, M_AESNI); - } + if (allocated) + zfree(buf, M_AESNI); + if (authallocated) + zfree(authbuf, M_AESNI); + if (outallocated) + zfree(outbuf, M_AESNI); explicit_bzero(iv, sizeof(iv)); explicit_bzero(tag, sizeof(tag)); return (error); diff --git a/sys/crypto/via/padlock_cipher.c b/sys/crypto/via/padlock_cipher.c index 863ad54454c4..1f3e8532e65f 100644 --- a/sys/crypto/via/padlock_cipher.c +++ b/sys/crypto/via/padlock_cipher.c @@ -234,8 +234,7 @@ padlock_cipher_process(struct padlock_session *ses, struct cryptop *crp, crypto_copyback(crp, crp->crp_payload_start, crp->crp_payload_length, abuf); - explicit_bzero(buf, crp->crp_payload_length + 16); - free(buf, M_PADLOCK); + zfree(buf, M_PADLOCK); } return (0); } diff --git a/sys/dev/cxgbe/crypto/t4_kern_tls.c b/sys/dev/cxgbe/crypto/t4_kern_tls.c index 29595c16568d..4102e11fa023 100644 --- a/sys/dev/cxgbe/crypto/t4_kern_tls.c +++ b/sys/dev/cxgbe/crypto/t4_kern_tls.c @@ -2337,8 +2337,7 @@ cxgbe_tls_tag_free(struct m_snd_tag *mst) if (tlsp->tx_key_addr >= 0) free_keyid(tlsp, tlsp->tx_key_addr); - explicit_bzero(&tlsp->keyctx, sizeof(&tlsp->keyctx)); - free(tlsp, M_CXGBE); + zfree(tlsp, M_CXGBE); } void diff --git a/sys/dev/random/randomdev.c b/sys/dev/random/randomdev.c index 785a4e16e4c9..363d91c37731 100644 --- a/sys/dev/random/randomdev.c +++ b/sys/dev/random/randomdev.c @@ -241,8 +241,7 @@ int if (error == ERESTART || error == EINTR) error = 0; - explicit_bzero(random_buf, bufsize); - free(random_buf, M_ENTROPY); + zfree(random_buf, M_ENTROPY); return (error); } diff --git a/sys/geom/eli/g_eli.c b/sys/geom/eli/g_eli.c index 8e7433f23594..52082294b4e3 100644 --- a/sys/geom/eli/g_eli.c +++ b/sys/geom/eli/g_eli.c @@ -388,10 +388,7 @@ g_eli_resize(struct g_consumer *cp) } iofail: explicit_bzero(&md, sizeof(md)); - if (sector != NULL) { - explicit_bzero(sector, pp->sectorsize); - free(sector, M_ELI); - } + zfree(sector, M_ELI); } oldsize = sc->sc_mediasize; diff --git a/sys/geom/eli/g_eli_ctl.c b/sys/geom/eli/g_eli_ctl.c index 64f4fa75bf75..0a8aea90afa0 100644 --- a/sys/geom/eli/g_eli_ctl.c +++ b/sys/geom/eli/g_eli_ctl.c @@ -655,8 +655,7 @@ g_eli_ctl_configure(struct gctl_req *req, struct g_class *mp) prov, error); } explicit_bzero(&md, sizeof(md)); - explicit_bzero(sector, pp->sectorsize); - free(sector, M_ELI); + zfree(sector, M_ELI); } } @@ -759,8 +758,7 @@ g_eli_ctl_setkey(struct gctl_req *req, struct g_class *mp) explicit_bzero(&md, sizeof(md)); error = g_write_data(cp, pp->mediasize - pp->sectorsize, sector, pp->sectorsize); - explicit_bzero(sector, pp->sectorsize); - free(sector, M_ELI); + zfree(sector, M_ELI); if (error != 0) { gctl_error(req, "Cannot store metadata on %s (error=%d).", pp->name, error); @@ -875,8 +873,7 @@ g_eli_ctl_delkey(struct gctl_req *req, struct g_class *mp) (void)g_io_flush(cp); } explicit_bzero(&md, sizeof(md)); - explicit_bzero(sector, pp->sectorsize); - free(sector, M_ELI); + zfree(sector, M_ELI); if (*all) G_ELI_DEBUG(1, "All keys removed from %s.", pp->name); else diff --git a/sys/geom/eli/g_eli_key_cache.c b/sys/geom/eli/g_eli_key_cache.c index db1b1a950a1e..cc60be89b815 100644 --- a/sys/geom/eli/g_eli_key_cache.c +++ b/sys/geom/eli/g_eli_key_cache.c @@ -118,8 +118,7 @@ g_eli_key_allocate(struct g_eli_softc *sc, uint64_t keyno) keysearch.gek_keyno = keyno; ekey = RB_FIND(g_eli_key_tree, &sc->sc_ekeys_tree, &keysearch); if (ekey != NULL) { - explicit_bzero(key, sizeof(*key)); - free(key, M_ELI); + zfree(key, M_ELI); key = ekey; TAILQ_REMOVE(&sc->sc_ekeys_queue, key, gek_next); } else { @@ -175,8 +174,7 @@ g_eli_key_remove(struct g_eli_softc *sc, struct g_eli_key *key) RB_REMOVE(g_eli_key_tree, &sc->sc_ekeys_tree, key); TAILQ_REMOVE(&sc->sc_ekeys_queue, key, gek_next); sc->sc_ekeys_allocated--; - explicit_bzero(key, sizeof(*key)); - free(key, M_ELI); + zfree(key, M_ELI); } void diff --git a/sys/geom/geom_dev.c b/sys/geom/geom_dev.c index 4c62da6feecc..da1d2d09c7c9 100644 --- a/sys/geom/geom_dev.c +++ b/sys/geom/geom_dev.c @@ -614,10 +614,7 @@ g_dev_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int fflag, struct thread kda->kda_encryptedkey = encryptedkey; error = g_dev_setdumpdev(dev, kda); } - if (encryptedkey != NULL) { - explicit_bzero(encryptedkey, kda->kda_encryptedkeysize); - free(encryptedkey, M_TEMP); - } + zfree(encryptedkey, M_TEMP); explicit_bzero(kda, sizeof(*kda)); break; } diff --git a/sys/kern/kern_environment.c b/sys/kern/kern_environment.c index f2d7dcb1de4d..312440ab509e 100644 --- a/sys/kern/kern_environment.c +++ b/sys/kern/kern_environment.c @@ -607,8 +607,7 @@ kern_unsetenv(const char *name) kenvp[i++] = kenvp[j]; kenvp[i] = NULL; mtx_unlock(&kenv_lock); - explicit_bzero(oldenv, strlen(oldenv)); - free(oldenv, M_KENV); + zfree(oldenv, M_KENV); return (0); } mtx_unlock(&kenv_lock); diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c index 9338b90b30f7..2a4951e1d3fe 100644 --- a/sys/kern/kern_shutdown.c +++ b/sys/kern/kern_shutdown.c @@ -1058,8 +1058,7 @@ kerneldumpcrypto_create(size_t blocksize, uint8_t encryption, return (kdc); failed: - explicit_bzero(kdc, sizeof(*kdc) + dumpkeysize); - free(kdc, M_EKCD); + zfree(kdc, M_EKCD); return (NULL); } @@ -1156,8 +1155,7 @@ kerneldumpcomp_destroy(struct dumperinfo *di) if (kdcomp == NULL) return; compressor_fini(kdcomp->kdc_stream); - explicit_bzero(kdcomp->kdc_buf, di->maxiosize); - free(kdcomp->kdc_buf, M_DUMPER); + zfree(kdcomp->kdc_buf, M_DUMPER); free(kdcomp, M_DUMPER); } @@ -1171,23 +1169,14 @@ free_single_dumper(struct dumperinfo *di) if (di == NULL) return; - if (di->blockbuf != NULL) { - explicit_bzero(di->blockbuf, di->blocksize); - free(di->blockbuf, M_DUMPER); - } + zfree(di->blockbuf, M_DUMPER); kerneldumpcomp_destroy(di); #ifdef EKCD - if (di->kdcrypto != NULL) { - explicit_bzero(di->kdcrypto, sizeof(*di->kdcrypto) + - di->kdcrypto->kdc_dumpkeysize); - free(di->kdcrypto, M_EKCD); - } + zfree(di->kdcrypto, M_EKCD); #endif - - explicit_bzero(di, sizeof(*di)); - free(di, M_DUMPER); + zfree(di, M_DUMPER); } /* Registration of dumpers */ diff --git a/sys/kern/uipc_ktls.c b/sys/kern/uipc_ktls.c index b9815750b141..4b3053b31494 100644 --- a/sys/kern/uipc_ktls.c +++ b/sys/kern/uipc_ktls.c @@ -682,15 +682,12 @@ ktls_cleanup(struct ktls_session *tls) #endif } if (tls->params.auth_key != NULL) { - explicit_bzero(tls->params.auth_key, tls->params.auth_key_len); - free(tls->params.auth_key, M_KTLS); + zfree(tls->params.auth_key, M_KTLS); tls->params.auth_key = NULL; tls->params.auth_key_len = 0; } if (tls->params.cipher_key != NULL) { - explicit_bzero(tls->params.cipher_key, - tls->params.cipher_key_len); - free(tls->params.cipher_key, M_KTLS); + zfree(tls->params.cipher_key, M_KTLS); tls->params.cipher_key = NULL; tls->params.cipher_key_len = 0; } diff --git a/sys/netinet/netdump/netdump_client.c b/sys/netinet/netdump/netdump_client.c index f2642cb5bc28..06a833c20c07 100644 --- a/sys/netinet/netdump/netdump_client.c +++ b/sys/netinet/netdump/netdump_client.c @@ -642,11 +642,7 @@ netdump_ioctl(struct cdev *dev __unused, u_long cmd, caddr_t addr, dumper.mediasize = 0; error = dumper_insert(&dumper, conf->kda_iface, conf); - if (encryptedkey != NULL) { - explicit_bzero(encryptedkey, - conf->kda_encryptedkeysize); - free(encryptedkey, M_TEMP); - } + zfree(encryptedkey, M_TEMP); if (error != 0) netdump_unconfigure(); break; diff --git a/sys/opencrypto/crypto.c b/sys/opencrypto/crypto.c index 6265d8f2c279..a14ee9812937 100644 --- a/sys/opencrypto/crypto.c +++ b/sys/opencrypto/crypto.c @@ -897,8 +897,7 @@ crypto_deletesession(crypto_session_t cses) cap = cses->cap; - explicit_bzero(cses->softc, cap->cc_session_size); - free(cses->softc, M_CRYPTO_DATA); + zfree(cses->softc, M_CRYPTO_DATA); uma_zfree(cryptoses_zone, cses); CRYPTO_DRIVER_LOCK(); diff --git a/sys/opencrypto/cryptosoft.c b/sys/opencrypto/cryptosoft.c index d55572a57d1f..27a6d472e3d8 100644 --- a/sys/opencrypto/cryptosoft.c +++ b/sys/opencrypto/cryptosoft.c @@ -1428,27 +1428,14 @@ static void swcr_freesession(device_t dev, crypto_session_t cses) { struct swcr_session *ses; - struct swcr_auth *swa; - struct auth_hash *axf; ses = crypto_get_driver_session(cses); mtx_destroy(&ses->swcr_lock); zfree(ses->swcr_encdec.sw_kschedule, M_CRYPTO_DATA); - - axf = ses->swcr_auth.sw_axf; - if (axf != NULL) { - swa = &ses->swcr_auth; - if (swa->sw_ictx != NULL) { - explicit_bzero(swa->sw_ictx, axf->ctxsize); - free(swa->sw_ictx, M_CRYPTO_DATA); - } - if (swa->sw_octx != NULL) { - explicit_bzero(swa->sw_octx, axf->ctxsize); - free(swa->sw_octx, M_CRYPTO_DATA); - } - } + zfree(ses->swcr_auth.sw_ictx, M_CRYPTO_DATA); + zfree(ses->swcr_auth.sw_octx, M_CRYPTO_DATA); } /* diff --git a/sys/opencrypto/ktls_ocf.c b/sys/opencrypto/ktls_ocf.c index a07d6c4dbfb2..867b68624f67 100644 --- a/sys/opencrypto/ktls_ocf.c +++ b/sys/opencrypto/ktls_ocf.c @@ -343,8 +343,7 @@ ktls_ocf_free(struct ktls_session *tls) os = tls->cipher; crypto_freesession(os->sid); mtx_destroy(&os->lock); - explicit_bzero(os, sizeof(*os)); - free(os, M_KTLS_OCF); + zfree(os, M_KTLS_OCF); } static int