Messages in this thread Patch in this message | | | Date | Sat, 30 Sep 2023 18:14:34 +0200 | From | Simon Horman <> | Subject | Re: [PATCH] ax25: Fix potential deadlock on &ax25_list_lock |
| |
On Tue, Sep 26, 2023 at 10:57:32AM +0000, Chengfeng Ye wrote: > Timer interrupt ax25_ds_timeout() could introduce double locks on > &ax25_list_lock. > > ax25_ioctl() > --> ax25_ctl_ioctl() > --> ax25_dama_off() > --> ax25_dev_dama_off() > --> ax25_check_dama_slave() > --> spin_lock(&ax25_list_lock) > <timer interrupt> > --> ax25_ds_timeout() > --> spin_lock(&ax25_list_lock) > > This flaw was found by an experimental static analysis tool I am > developing for irq-related deadlock. > > To prevent the potential deadlock, the patch use spin_lock_bh() > on &ax25_list_lock inside ax25_check_dama_slave(). > > Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
Hi Chengfeng Ye,
thanks for your patch.
As a fix for Networking this should probably be targeted at the 'net' tree. Which should be denoted in the subject.
Subject: [PATCH net] ...
And as a fix this patch should probably have a Fixes tag. This ones seem appropriate to me, but I could be wrong.
Fixes: c070e51db5e2 ("ice: always add legacy 32byte RXDID in supported_rxdids")
I don't think it is necessary to repost just to address these issues, but the Networking maintainers may think otherwise.
The code change itself looks good to me.
Reviewed-by: Simon Horman <horms@kernel.org>
> --- > net/ax25/ax25_ds_subr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/ax25/ax25_ds_subr.c b/net/ax25/ax25_ds_subr.c > index f00e27df3c76..010b11303d32 100644 > --- a/net/ax25/ax25_ds_subr.c > +++ b/net/ax25/ax25_ds_subr.c > @@ -156,13 +156,13 @@ static int ax25_check_dama_slave(ax25_dev *ax25_dev) > ax25_cb *ax25; > int res = 0; > > - spin_lock(&ax25_list_lock); > + spin_lock_bh(&ax25_list_lock); > ax25_for_each(ax25, &ax25_list) > if (ax25->ax25_dev == ax25_dev && (ax25->condition & AX25_COND_DAMA_MODE) && ax25->state > AX25_STATE_1) { > res = 1; > break; > } > - spin_unlock(&ax25_list_lock); > + spin_unlock_bh(&ax25_list_lock); > > return res; > } > -- > 2.17.1 > >
| |