netdev - Re: [PATCH net-next 2/2] Crypto/chcr: Checking cra_refcnt before unregistering the algorithms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Jun 2020 13:48:12 +1000
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     Ayush Sawal <ayush.sawal@...lsio.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        manojmalviya@...lsio.com
Subject: Re: [PATCH net-next 2/2] Crypto/chcr: Checking cra_refcnt before
 unregistering the algorithms

On Wed, Jun 10, 2020 at 02:54:32AM +0530, Ayush Sawal wrote:
> This patch puts a check for algorithm unregister, to avoid removal of
> driver if the algorithm is under use.
> 
> Signed-off-by: Ayush Sawal <ayush.sawal@...lsio.com>
> ---
>  drivers/crypto/chelsio/chcr_algo.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/chelsio/chcr_algo.c b/drivers/crypto/chelsio/chcr_algo.c
> index f8b55137cf7d..4c2553672b6f 100644
> --- a/drivers/crypto/chelsio/chcr_algo.c
> +++ b/drivers/crypto/chelsio/chcr_algo.c
> @@ -4391,22 +4391,32 @@ static int chcr_unregister_alg(void)
>  	for (i = 0; i < ARRAY_SIZE(driver_algs); i++) {
>  		switch (driver_algs[i].type & CRYPTO_ALG_TYPE_MASK) {
>  		case CRYPTO_ALG_TYPE_SKCIPHER:
> -			if (driver_algs[i].is_registered)
> +			if (driver_algs[i].is_registered && refcount_read(
> +			    &driver_algs[i].alg.skcipher.base.cra_refcnt)
> +			    == 1) {
>  				crypto_unregister_skcipher(
>  						&driver_algs[i].alg.skcipher);
> +				driver_algs[i].is_registered = 0;
> +			}

This is wrong.  cra_refcnt must not be used directly by drivers.

Normally driver unregister is stopped by the module reference
count.  This is not the case for your driver because of the existence
of a path of unregistration that is not tied to module removal.

To support that properly, we need to add code to the Crypto API
to handle this, as opposed to adding hacks to the driver.

Thanks,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt