linux-kernel - Re: [PATCH] crypto: iaa - Fix race condition when probing IAA devices

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: <878qlyugea.fsf@intel.com>
Date: Wed, 11 Jun 2025 14:17:49 -0700
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: Kristen Accardi <kristen.c.accardi@...el.com>, "David S. Miller"
 <davem@...emloft.net>, Tom Zanussi <tom.zanussi@...ux.intel.com>,
 linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] crypto: iaa - Fix race condition when probing IAA devices

Herbert Xu <herbert@...dor.apana.org.au> writes:

> On Tue, Jun 03, 2025 at 04:55:31PM -0700, Vinicius Costa Gomes wrote:
>>
>> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
>> index 23f585219fb4..2185c101bef3 100644
>> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
>> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
>> @@ -35,28 +35,39 @@ static unsigned int cpus_per_iaa;
>>  
>>  /* Per-cpu lookup table for balanced wqs */
>>  static struct wq_table_entry __percpu *wq_table;
>> +static DEFINE_SPINLOCK(wq_table_lock);
>
> This can be called in BH context so you need to disable BH when
> taking the spinlock.
>

My tests with lockdep enabled must have missed something, will verify
and fix this.

>>  static struct idxd_wq *wq_table_next_wq(int cpu)
>>  {
>> -	struct wq_table_entry *entry = per_cpu_ptr(wq_table, cpu);
>> +	struct wq_table_entry *entry;
>> +	struct idxd_wq *wq;
>> +	int id;
>> +
>> +	guard(spinlock)(&wq_table_lock);
>> +
>> +	entry = per_cpu_ptr(wq_table, cpu);
>
> You're taking a global spinlock around a per-cpu variable.  In that
> case you might as well get rid of the per-cpu variable and use the
> spinlock only.
>

>From what I could gather, the idea of the per-cpu workqueue table ("map"
really) is more to "spread" the workqueues to different CPUS than to
reduce contention.

If the question is more about the choice of using per-cpu variables, I
can look for alternatives.

> Cheers,
> -- 
> 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

Cheers,
-- 
Vinicius