linux-kernel - Re: [PATCH v4 03/16] 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: <VI1PR04MB505565EC5520F4820E234A84EEFB0@VI1PR04MB5055.eurprd04.prod.outlook.com>
Date:   Wed, 3 Jul 2019 13:51:16 +0000
From:   Leonard Crestez <leonard.crestez@....com>
To:     Andrey Smirnov <andrew.smirnov@...il.com>,
        Horia Geanta <horia.geanta@....com>
CC:     "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        Chris Spencer <christopher.spencer@....co.uk>,
        Cory Tusar <cory.tusar@....aero>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Aymen Sghaier <aymen.sghaier@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 03/16] crypto: caam - move tasklet_init() call down

On 7/3/2019 11:14 AM, Andrey Smirnov wrote:
> Move tasklet_init() call further down in order to simplify error path
> cleanup. No functional change intended.
> 
> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> index 4b25b2fa3d02..a7ca2bbe243f 100644
> --- a/drivers/crypto/caam/jr.c
> +++ b/drivers/crypto/caam/jr.c
> @@ -441,15 +441,13 @@ static int caam_jr_init(struct device *dev)
>   
>   	jrp = dev_get_drvdata(dev);
>   
> -	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
> -
>   	/* Connect job ring interrupt handler. */
>   	error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
>   			    dev_name(dev), dev);
>   	if (error) {
>   		dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
>   			jrp->ridx, jrp->irq);
> -		goto out_kill_deq;
> +		return error;
>   	}

The caam_jr_interrupt handler can schedule the tasklet so it makes sense 
to have it be initialized ahead of request_irq. In theory it's possible 
for an interrupt to be triggered immediately when request_irq is called.

I'm not very familiar with the CAAM ip, can you ensure no interrupts are 
pending in HW at probe time? The "no functional change" part is not obvious.

--
Regards,
Leonard