linux-kernel - RE: [EXT] [PATCH] scsi: qedi: Fix potential deadlock on &qedi_percpu->p_work

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]

Date:   Mon, 7 Aug 2023 12:20:35 +0000
From:   Manish Rangankar <mrangankar@...vell.com>
To:     Chengfeng Ye <dg573847474@...il.com>,
        Nilesh Javali <njavali@...vell.com>,
        GR-QLogic-Storage-Upstream <GR-QLogic-Storage-Upstream@...vell.com>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>
CC:     "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] [PATCH] scsi: qedi: Fix potential deadlock on
 &qedi_percpu->p_work_lock


> -----Original Message-----
> From: Chengfeng Ye <dg573847474@...il.com>
> Sent: Wednesday, July 26, 2023 6:27 PM
> To: Nilesh Javali <njavali@...vell.com>; Manish Rangankar
> <mrangankar@...vell.com>; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@...vell.com>; jejb@...ux.ibm.com;
> martin.petersen@...cle.com
> Cc: linux-scsi@...r.kernel.org; linux-kernel@...r.kernel.org; Chengfeng Ye
> <dg573847474@...il.com>
> Subject: [EXT] [PATCH] scsi: qedi: Fix potential deadlock on &qedi_percpu-
> >p_work_lock
> 
> External Email
> 
> ----------------------------------------------------------------------
> As &qedi_percpu->p_work_lock is acquired by hard irq qedi_msix_handler(),
> other acquisition of the same lock under process context should disable irq,
> otherwise deadlock could happen if the irq preempt the execution while the
> lock is held in process context on the same CPU.
> 
> qedi_cpu_offline() is one such function acquires the lock on process context.
> 
> [Deadlock Scenario]
> qedi_cpu_offline()
>     ->spin_lock(&p->p_work_lock)
>         <irq>
>         ->qedi_msix_handler()
>         ->edi_process_completions()
>         ->spin_lock_irqsave(&p->p_work_lock, flags); (deadlock here)
> 
> This flaw was found by an experimental static analysis tool I am developing
> for irq-related deadlock.
> 
> The tentative patch fix the potential deadlock by spin_lock_irqsave() under
> process context.
> 
> Signed-off-by: Chengfeng Ye <dg573847474@...il.com>
> ---
>  drivers/scsi/qedi/qedi_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
> index 450522b204d6..77a56a136678 100644
> --- a/drivers/scsi/qedi/qedi_main.c
> +++ b/drivers/scsi/qedi/qedi_main.c
> @@ -1976,8 +1976,9 @@ static int qedi_cpu_offline(unsigned int cpu)
>  	struct qedi_percpu_s *p = this_cpu_ptr(&qedi_percpu);
>  	struct qedi_work *work, *tmp;
>  	struct task_struct *thread;
> +	unsigned long flags;
> 
> -	spin_lock_bh(&p->p_work_lock);
> +	spin_lock_irqsave(&p->p_work_lock, flags);
>  	thread = p->iothread;
>  	p->iothread = NULL;
> 
> @@ -1988,7 +1989,7 @@ static int qedi_cpu_offline(unsigned int cpu)
>  			kfree(work);
>  	}
> 
> -	spin_unlock_bh(&p->p_work_lock);
> +	spin_unlock_irqrestore(&p->p_work_lock, flags);
>  	if (thread)
>  		kthread_stop(thread);
>  	return 0;
> --
> 2.17.1


Thanks for the patch,

Acked-by: Manish Rangankar <mrangankar@...vell.com>