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
| ||
|
Message-ID: <0ce8658b-9b8a-923c-a5bd-c63284d2db96@intel.com> Date: Fri, 29 Sep 2023 11:29:50 -0700 From: Jacob Keller <jacob.e.keller@...el.com> To: Chengfeng Ye <dg573847474@...il.com>, <jmaloy@...hat.com>, <ying.xue@...driver.com>, <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com> CC: <netdev@...r.kernel.org>, <tipc-discussion@...ts.sourceforge.net>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] tipc: fix a potential deadlock on &tx->lock On 9/27/2023 11:14 AM, Chengfeng Ye wrote: > It seems that tipc_crypto_key_revoke() could be be invoked by > wokequeue tipc_crypto_work_rx() under process context and > timer/rx callback under softirq context, thus the lock acquisition > on &tx->lock seems better use spin_lock_bh() to prevent possible > deadlock. > > This flaw was found by an experimental static analysis tool I am > developing for irq-related deadlock. > Interesting. > tipc_crypto_work_rx() <workqueue> > --> tipc_crypto_key_distr() > --> tipc_bcast_xmit() > --> tipc_bcbase_xmit() > --> tipc_bearer_bc_xmit() > --> tipc_crypto_xmit() > --> tipc_ehdr_build() > --> tipc_crypto_key_revoke() > --> spin_lock(&tx->lock) > <timer interrupt> > --> tipc_disc_timeout() > --> tipc_bearer_xmit_skb() > --> tipc_crypto_xmit() > --> tipc_ehdr_build() > --> tipc_crypto_key_revoke() > --> spin_lock(&tx->lock) <deadlock here> > > Signed-off-by: Chengfeng Ye <dg573847474@...il.com> > --- Reviewed-by: Jacob Keller <jacob.e.keller@...el.com> > net/tipc/crypto.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c > index 302fd749c424..43c3f1c971b8 100644 > --- a/net/tipc/crypto.c > +++ b/net/tipc/crypto.c > @@ -1441,14 +1441,14 @@ static int tipc_crypto_key_revoke(struct net *net, u8 tx_key) > struct tipc_crypto *tx = tipc_net(net)->crypto_tx; > struct tipc_key key; > > - spin_lock(&tx->lock); > + spin_lock_bh(&tx->lock); > key = tx->key; > WARN_ON(!key.active || tx_key != key.active); > > /* Free the active key */ > tipc_crypto_key_set_state(tx, key.passive, 0, key.pending); > tipc_crypto_key_detach(tx->aead[key.active], &tx->lock); > - spin_unlock(&tx->lock); > + spin_unlock_bh(&tx->lock); > > pr_warn("%s: key is revoked\n", tx->name); > return -EKEYREVOKED;
Powered by blists - more mailing lists