linux-kernel - Re: [PATCH 4/4] crypto: s5p-sss - Use mutex instead of spinlock

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: <4289522.7Nbtc4pSSm@amdc3058>
Date:   Fri, 17 Mar 2017 18:28:29 +0100
From:   Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
        Vladimir Zapolskiy <vz@...ia.com>,
        Nathan Royce <nroycea+kernel@...il.com>
Subject: Re: [PATCH 4/4] crypto: s5p-sss - Use mutex instead of spinlock


Hi,

On Friday, March 17, 2017 04:49:22 PM Krzysztof Kozlowski wrote:
> Driver uses threaded interrupt handler so there is no real need for
> using spinlocks for synchronization.  Mutexes would do fine and are
> friendlier for overall system preemptivness and real-time behavior.

Are you sure that this conversion is safe?  This driver also uses
a tasklet and tasklets run in the interrupt context.

> @@ -667,18 +666,17 @@ static void s5p_tasklet_cb(unsigned long data)
>  	struct s5p_aes_dev *dev = (struct s5p_aes_dev *)data;
>  	struct crypto_async_request *async_req, *backlog;
>  	struct s5p_aes_reqctx *reqctx;
> -	unsigned long flags;
>  
> -	spin_lock_irqsave(&dev->lock, flags);
> +	mutex_lock(&dev->lock);
>  	backlog   = crypto_get_backlog(&dev->queue);
>  	async_req = crypto_dequeue_request(&dev->queue);
>  
>  	if (!async_req) {
>  		dev->busy = false;
> -		spin_unlock_irqrestore(&dev->lock, flags);
> +		mutex_unlock(&dev->lock);
>  		return;
>  	}
> -	spin_unlock_irqrestore(&dev->lock, flags);
> +	mutex_unlock(&dev->lock);

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics