linux-kernel - Re: [PATCH v2] padata: use smp_mb in padata

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: <20190718054201.jtfcwkygunyvgwe4@gondor.apana.org.au>
Date:   Thu, 18 Jul 2019 13:42:01 +0800
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     Daniel Jordan <daniel.m.jordan@...cle.com>
Cc:     Steffen Klassert <steffen.klassert@...unet.com>,
        Andrea Parri <andrea.parri@...rulasolutions.com>,
        Boqun Feng <boqun.feng@...il.com>,
        "Paul E . McKenney" <paulmck@...ux.ibm.com>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-arch@...r.kernel.org, linux-crypto@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] padata: use smp_mb in padata_reorder to avoid
 orphaned padata jobs

On Tue, Jul 16, 2019 at 12:32:53PM -0400, Daniel Jordan wrote:
> Testing padata with the tcrypt module on a 5.2 kernel...
> 
>     # modprobe tcrypt alg="pcrypt(rfc4106(gcm(aes)))" type=3
>     # modprobe tcrypt mode=211 sec=1
> 
> ...produces this splat:
> 
>     INFO: task modprobe:10075 blocked for more than 120 seconds.
>           Not tainted 5.2.0-base+ #16
>     modprobe        D    0 10075  10064 0x80004080
>     Call Trace:
>      ? __schedule+0x4dd/0x610
>      ? ring_buffer_unlock_commit+0x23/0x100
>      schedule+0x6c/0x90
>      schedule_timeout+0x3b/0x320
>      ? trace_buffer_unlock_commit_regs+0x4f/0x1f0
>      wait_for_common+0x160/0x1a0
>      ? wake_up_q+0x80/0x80
>      { crypto_wait_req }             # entries in braces added by hand
>      { do_one_aead_op }
>      { test_aead_jiffies }
>      test_aead_speed.constprop.17+0x681/0xf30 [tcrypt]
>      do_test+0x4053/0x6a2b [tcrypt]
>      ? 0xffffffffa00f4000
>      tcrypt_mod_init+0x50/0x1000 [tcrypt]
>      ...
> 
> The second modprobe command never finishes because in padata_reorder,
> CPU0's load of reorder_objects is executed before the unlocking store in
> spin_unlock_bh(pd->lock), causing CPU0 to miss CPU1's increment:
> 
> CPU0                                 CPU1
> 
> padata_reorder                       padata_do_serial
>   LOAD reorder_objects  // 0
>                                        INC reorder_objects  // 1
>                                        padata_reorder
>                                          TRYLOCK pd->lock   // failed
>   UNLOCK pd->lock
> 
> CPU0 deletes the timer before returning from padata_reorder and since no
> other job is submitted to padata, modprobe waits indefinitely.
> 
> Add a pair of full barriers to guarantee proper ordering:
> 
> CPU0                                 CPU1
> 
> padata_reorder                       padata_do_serial
>   UNLOCK pd->lock
>   smp_mb()
>   LOAD reorder_objects
>                                        INC reorder_objects
>                                        smp_mb__after_atomic()
>                                        padata_reorder
>                                          TRYLOCK pd->lock
> 
> smp_mb__after_atomic is needed so the read part of the trylock operation
> comes after the INC, as Andrea points out.   Thanks also to Andrea for
> help with writing a litmus test.
> 
> Fixes: 16295bec6398 ("padata: Generic parallelization/serialization interface")
> Signed-off-by: Daniel Jordan <daniel.m.jordan@...cle.com>
> Cc: Andrea Parri <andrea.parri@...rulasolutions.com>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: Herbert Xu <herbert@...dor.apana.org.au>
> Cc: Paul E. McKenney <paulmck@...ux.ibm.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Steffen Klassert <steffen.klassert@...unet.com>
> Cc: linux-arch@...r.kernel.org
> Cc: linux-crypto@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> ---
>  kernel/padata.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Patch applied.  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