From 3877526a1e044021de4c42aba89655a6a5889f16 Mon Sep 17 00:00:00 2001 From: bluhm Date: Wed, 13 Oct 2021 13:08:58 +0000 Subject: [PATCH] The kernel crypto framework sometimes returned an error, sometimes the callback was called, and sometimes both. So the caller of that API could not release resources correctly. A bunch of errors can or should not happen, replace them with an assert. Remove redundant checks. crypto_invoke() should not return the error, but pass it via callback. Some old hardware drivers keep part of their inconsistency as I cannot test them. OK mpi@ --- sys/arch/amd64/amd64/aesni.c | 7 ++----- sys/arch/amd64/amd64/via.c | 7 ++----- sys/arch/arm64/arm64/cryptox.c | 7 ++----- sys/arch/i386/i386/via.c | 7 ++----- sys/arch/i386/pci/glxsb.c | 11 ++--------- sys/arch/octeon/dev/octcrypto.c | 5 +---- sys/crypto/crypto.c | 18 ++++++++---------- sys/crypto/cryptodev.h | 4 ++-- sys/crypto/cryptosoft.c | 8 +++----- sys/dev/pci/hifn7751.c | 7 +------ sys/dev/pci/safe.c | 8 ++------ sys/dev/pci/ubsec.c | 6 +----- 12 files changed, 28 insertions(+), 67 deletions(-) diff --git a/sys/arch/amd64/amd64/aesni.c b/sys/arch/amd64/amd64/aesni.c index d2e0aa14028..12ed40b37a8 100644 --- a/sys/arch/amd64/amd64/aesni.c +++ b/sys/arch/amd64/amd64/aesni.c @@ -1,4 +1,4 @@ -/* $OpenBSD: aesni.c,v 1.50 2021/07/08 09:22:30 bluhm Exp $ */ +/* $OpenBSD: aesni.c,v 1.51 2021/10/13 13:08:58 bluhm Exp $ */ /*- * Copyright (c) 2003 Jason Wright * Copyright (c) 2003, 2004 Theo de Raadt @@ -639,10 +639,7 @@ aesni_process(struct cryptop *crp) int err = 0; int i; - if (crp == NULL || crp->crp_callback == NULL) - return (EINVAL); - if (crp->crp_ndesc < 1) - return (EINVAL); + KASSERT(crp->crp_ndesc >= 1); smr_read_enter(); ses = aesni_get(crp->crp_sid & 0xffffffff); diff --git a/sys/arch/amd64/amd64/via.c b/sys/arch/amd64/amd64/via.c index c2b162457d7..d6f04c3a88c 100644 --- a/sys/arch/amd64/amd64/via.c +++ b/sys/arch/amd64/amd64/via.c @@ -1,4 +1,4 @@ -/* $OpenBSD: via.c,v 1.34 2021/07/08 09:22:30 bluhm Exp $ */ +/* $OpenBSD: via.c,v 1.35 2021/10/13 13:08:58 bluhm Exp $ */ /* $NetBSD: machdep.c,v 1.214 1996/11/10 03:16:17 thorpej Exp $ */ /*- @@ -420,10 +420,7 @@ viac3_crypto_process(struct cryptop *crp) int sesn, err = 0; int i; - if (crp == NULL || crp->crp_callback == NULL) - return (EINVAL); - if (crp->crp_ndesc < 1) - return (EINVAL); + KASSERT(crp->crp_ndesc >= 1); sesn = VIAC3_SESSION(crp->crp_sid); if (sesn >= sc->sc_nsessions) { diff --git a/sys/arch/arm64/arm64/cryptox.c b/sys/arch/arm64/arm64/cryptox.c index ab18895752d..0b4ff183f04 100644 --- a/sys/arch/arm64/arm64/cryptox.c +++ b/sys/arch/arm64/arm64/cryptox.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cryptox.c,v 1.2 2021/07/08 09:22:30 bluhm Exp $ */ +/* $OpenBSD: cryptox.c,v 1.3 2021/10/13 13:08:58 bluhm Exp $ */ /* * Copyright (c) 2003 Jason Wright * Copyright (c) 2003, 2004 Theo de Raadt @@ -447,10 +447,7 @@ cryptox_process(struct cryptop *crp) int err = 0; int i; - if (crp == NULL || crp->crp_callback == NULL) - return (EINVAL); - if (crp->crp_ndesc < 1) - return (EINVAL); + KASSERT(crp->crp_ndesc >= 1); smr_read_enter(); ses = cryptox_get(crp->crp_sid & 0xffffffff); diff --git a/sys/arch/i386/i386/via.c b/sys/arch/i386/i386/via.c index dd389d29323..9822bec8519 100644 --- a/sys/arch/i386/i386/via.c +++ b/sys/arch/i386/i386/via.c @@ -1,4 +1,4 @@ -/* $OpenBSD: via.c,v 1.47 2021/07/08 09:22:30 bluhm Exp $ */ +/* $OpenBSD: via.c,v 1.48 2021/10/13 13:08:58 bluhm Exp $ */ /* $NetBSD: machdep.c,v 1.214 1996/11/10 03:16:17 thorpej Exp $ */ /*- @@ -428,10 +428,7 @@ viac3_crypto_process(struct cryptop *crp) int sesn, err = 0; int i; - if (crp == NULL || crp->crp_callback == NULL) - return (EINVAL); - if (crp->crp_ndesc < 1) - return (EINVAL); + KASSERT(crp->crp_ndesc >= 1); sesn = VIAC3_SESSION(crp->crp_sid); if (sesn >= sc->sc_nsessions) { diff --git a/sys/arch/i386/pci/glxsb.c b/sys/arch/i386/pci/glxsb.c index 6cdb3034d25..2dca0de55fb 100644 --- a/sys/arch/i386/pci/glxsb.c +++ b/sys/arch/i386/pci/glxsb.c @@ -1,4 +1,4 @@ -/* $OpenBSD: glxsb.c,v 1.37 2021/07/08 09:22:30 bluhm Exp $ */ +/* $OpenBSD: glxsb.c,v 1.38 2021/10/13 13:08:58 bluhm Exp $ */ /* * Copyright (c) 2006 Tom Cosgrove @@ -780,14 +780,7 @@ glxsb_crypto_process(struct cryptop *crp) s = splnet(); - if (crp == NULL || crp->crp_callback == NULL) { - err = EINVAL; - goto out; - } - if (crp->crp_ndesc < 1) { - err = EINVAL; - goto out; - } + KASSERT(crp->crp_ndesc >= 1); sesn = GLXSB_SESSION(crp->crp_sid); if (sesn >= sc->sc_nsessions) { diff --git a/sys/arch/octeon/dev/octcrypto.c b/sys/arch/octeon/dev/octcrypto.c index abfa258dce2..26449679790 100644 --- a/sys/arch/octeon/dev/octcrypto.c +++ b/sys/arch/octeon/dev/octcrypto.c @@ -1,4 +1,4 @@ -/* $OpenBSD: octcrypto.c,v 1.5 2021/07/08 09:22:30 bluhm Exp $ */ +/* $OpenBSD: octcrypto.c,v 1.6 2021/10/13 13:08:58 bluhm Exp $ */ /* * Copyright (c) 2018 Visa Hankala @@ -597,9 +597,6 @@ octcrypto_process(struct cryptop *crp) int error = 0; int i; - if (crp == NULL || crp->crp_callback == NULL) - return EINVAL; - KASSERT(crp->crp_ndesc >= 1); smr_read_enter(); diff --git a/sys/crypto/crypto.c b/sys/crypto/crypto.c index bff6dd4a97c..a278f305ea3 100644 --- a/sys/crypto/crypto.c +++ b/sys/crypto/crypto.c @@ -1,4 +1,4 @@ -/* $OpenBSD: crypto.c,v 1.85 2021/07/26 21:27:56 bluhm Exp $ */ +/* $OpenBSD: crypto.c,v 1.86 2021/10/13 13:08:58 bluhm Exp $ */ /* * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu) * @@ -404,7 +404,7 @@ crypto_dispatch(struct cryptop *crp) if (crp->crp_flags & CRYPTO_F_NOQUEUE) { if (lock) KERNEL_LOCK(); - error = crypto_invoke(crp); + crypto_invoke(crp); if (lock) KERNEL_UNLOCK(); } else { @@ -421,7 +421,7 @@ crypto_dispatch(struct cryptop *crp) /* * Dispatch a crypto request to the appropriate crypto devices. */ -int +void crypto_invoke(struct cryptop *crp) { u_int64_t nid; @@ -430,17 +430,15 @@ crypto_invoke(struct cryptop *crp) int s, i; /* Sanity checks. */ - if (crp == NULL || crp->crp_callback == NULL) - return EINVAL; + KASSERT(crp != NULL); + KASSERT(crp->crp_callback != NULL); KERNEL_ASSERT_LOCKED(); s = splvm(); if (crp->crp_ndesc < 1 || crypto_drivers == NULL) { crp->crp_etype = EINVAL; - crypto_done(crp); - splx(s); - return 0; + goto done; } hid = (crp->crp_sid >> 32) & 0xffffffff; @@ -470,7 +468,7 @@ crypto_invoke(struct cryptop *crp) } splx(s); - return 0; + return; migrate: /* Migrate session. */ @@ -482,9 +480,9 @@ crypto_invoke(struct cryptop *crp) crp->crp_sid = nid; crp->crp_etype = EAGAIN; + done: crypto_done(crp); splx(s); - return 0; } /* diff --git a/sys/crypto/cryptodev.h b/sys/crypto/cryptodev.h index 10bb1a6f625..699074db83f 100644 --- a/sys/crypto/cryptodev.h +++ b/sys/crypto/cryptodev.h @@ -1,4 +1,4 @@ -/* $OpenBSD: cryptodev.h,v 1.74 2021/07/26 21:27:56 bluhm Exp $ */ +/* $OpenBSD: cryptodev.h,v 1.75 2021/10/13 13:08:58 bluhm Exp $ */ /* * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu) @@ -224,7 +224,7 @@ int crypto_register(u_int32_t, int *, int (*)(struct cryptop *)); int crypto_unregister(u_int32_t, int); int32_t crypto_get_driverid(u_int8_t); -int crypto_invoke(struct cryptop *); +void crypto_invoke(struct cryptop *); void crypto_done(struct cryptop *); void cuio_copydata(struct uio *, int, int, caddr_t); diff --git a/sys/crypto/cryptosoft.c b/sys/crypto/cryptosoft.c index 6fef68ac276..3fc7e3ca8f0 100644 --- a/sys/crypto/cryptosoft.c +++ b/sys/crypto/cryptosoft.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cryptosoft.c,v 1.88 2021/07/09 15:29:55 bluhm Exp $ */ +/* $OpenBSD: cryptosoft.c,v 1.89 2021/10/13 13:08:58 bluhm Exp $ */ /* * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu) @@ -1035,11 +1035,9 @@ swcr_process(struct cryptop *crp) int type; int i; - /* Sanity check */ - if (crp == NULL) - return EINVAL; + KASSERT(crp->crp_ndesc >= 1); - if (crp->crp_ndesc < 1 || crp->crp_buf == NULL) { + if (crp->crp_buf == NULL) { crp->crp_etype = EINVAL; goto done; } diff --git a/sys/dev/pci/hifn7751.c b/sys/dev/pci/hifn7751.c index 9e0ef43142d..cae21a06000 100644 --- a/sys/dev/pci/hifn7751.c +++ b/sys/dev/pci/hifn7751.c @@ -1,4 +1,4 @@ -/* $OpenBSD: hifn7751.c,v 1.180 2020/05/29 04:42:25 deraadt Exp $ */ +/* $OpenBSD: hifn7751.c,v 1.181 2021/10/13 13:08:58 bluhm Exp $ */ /* * Invertex AEON / Hifn 7751 driver @@ -1925,11 +1925,6 @@ hifn_process(struct cryptop *crp) struct hifn_softc *sc; struct cryptodesc *crd1, *crd2 = NULL, *maccrd, *enccrd; - if (crp == NULL || crp->crp_callback == NULL) { - hifnstats.hst_invalid++; - return (EINVAL); - } - if (crp->crp_ilen == 0) { err = EINVAL; goto errout; diff --git a/sys/dev/pci/safe.c b/sys/dev/pci/safe.c index 639d44f6455..d95b0b3c8b6 100644 --- a/sys/dev/pci/safe.c +++ b/sys/dev/pci/safe.c @@ -1,4 +1,4 @@ -/* $OpenBSD: safe.c,v 1.45 2021/02/25 02:48:20 dlg Exp $ */ +/* $OpenBSD: safe.c,v 1.46 2021/10/13 13:08:58 bluhm Exp $ */ /*- * Copyright (c) 2003 Sam Leffler, Errno Consulting @@ -310,11 +310,7 @@ safe_process(struct cryptop *crp) u_int32_t cmd0, cmd1, staterec, iv[4]; s = splnet(); - if (crp == NULL || crp->crp_callback == NULL) { - safestats.st_invalid++; - splx(s); - return (EINVAL); - } + card = SAFE_CARD(crp->crp_sid); if (card >= safe_cd.cd_ndevs || safe_cd.cd_devs[card] == NULL) { safestats.st_invalid++; diff --git a/sys/dev/pci/ubsec.c b/sys/dev/pci/ubsec.c index aedc05eec2e..e52cd5911e9 100644 --- a/sys/dev/pci/ubsec.c +++ b/sys/dev/pci/ubsec.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ubsec.c,v 1.167 2021/02/25 02:48:20 dlg Exp $ */ +/* $OpenBSD: ubsec.c,v 1.168 2021/10/13 13:08:58 bluhm Exp $ */ /* * Copyright (c) 2000 Jason L. Wright (jason@thought.net) @@ -781,10 +781,6 @@ ubsec_process(struct cryptop *crp) u_int16_t flags = 0; int ivlen = 0, keylen = 0; - if (crp == NULL || crp->crp_callback == NULL) { - ubsecstats.hst_invalid++; - return (EINVAL); - } card = UBSEC_CARD(crp->crp_sid); if (card >= ubsec_cd.cd_ndevs || ubsec_cd.cd_devs[card] == NULL) { ubsecstats.hst_invalid++; -- 2.20.1