linux-kernel - Re: [PATCH v7 6/9] crypto: caam

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]

Message-ID: <VI1PR0402MB34857006B42E8F0DAFECC514981B0@VI1PR0402MB3485.eurprd04.prod.outlook.com>
Date:   Wed, 12 Feb 2020 10:41:21 +0000
From:   Horia Geanta <horia.geanta@....com>
To:     Andrey Smirnov <andrew.smirnov@...il.com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>
CC:     Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Iuliana Prodan <iuliana.prodan@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH v7 6/9] crypto: caam - check if RNG job failed

On 1/27/2020 6:57 PM, Andrey Smirnov wrote:
> @@ -60,12 +65,12 @@ static struct caam_rng_ctx *to_caam_rng_ctx(struct hwrng *r)
>  static void caam_rng_done(struct device *jrdev, u32 *desc, u32 err,
>  			  void *context)
>  {
> -	struct completion *done = context;
> +	struct caam_rng_job_ctx *jctx = context;
>  
>  	if (err)
> -		caam_jr_strstatus(jrdev, err);
> +		*jctx->err = caam_jr_strstatus(jrdev, err);
>  
> -	complete(done);
> +	complete(jctx->done);
>  }
>  
>  static u32 *caam_init_desc(u32 *desc, dma_addr_t dst_dma, int len)
> @@ -89,6 +94,10 @@ static int caam_rng_read_one(struct device *jrdev,
>  {
>  	dma_addr_t dst_dma;
>  	int err;
> +	struct caam_rng_job_ctx jctx = {
> +		.done = done,
> +		.err  = &err,
> +	};
>  
>  	len = min_t(int, len, CAAM_RNG_MAX_FIFO_STORE_SIZE);
>  
> @@ -101,7 +110,7 @@ static int caam_rng_read_one(struct device *jrdev,
>  	init_completion(done);
>  	err = caam_jr_enqueue(jrdev,
>  			      caam_init_desc(desc, dst_dma, len),
> -			      caam_rng_done, done);
> +			      caam_rng_done, &jctx);
AFAICT there's a race condition b/w caam_jr_enqueue() and caam_rng_done(),
both writing to "err":
caam_jr_enqueue()
	-> JR interrupt -> caam_jr_interrupt() -> tasklet_schedule()...
	-> spin_unlock_bh()
	-> caam_jr_dequeue() -> caam_rng_done() -> write err
	-> return 0 -> write err

Horia