Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Manish Rangankar <mrangankar@marvell.com>
To:     Chengfeng Ye <dg573847474@gmail.com>,
        Nilesh Javali <njavali@marvell.com>,
        GR-QLogic-Storage-Upstream <GR-QLogic-Storage-Upstream@marvell.com>,
        "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>
CC:     "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [EXT] [PATCH] scsi: qedi: Fix potential deadlock on
 &qedi_percpu->p_work_lock
Thread-Topic: [EXT] [PATCH] scsi: qedi: Fix potential deadlock on
 &qedi_percpu->p_work_lock
Thread-Index: AQHZv8CvXtqFAcAlF02PG4Hr7B2ow6/e01zA
Date:   Mon, 7 Aug 2023 12:20:35 +0000
Message-ID: <CO6PR18MB4419302FE56A3EE7605DF871D80CA@CO6PR18MB4419.namprd18.prod.outlook.com>
References: <20230726125655.4197-1-dg573847474@gmail.com>
In-Reply-To: <20230726125655.4197-1-dg573847474@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?JpcVMsf3qqwdqrGWqHR/jVE4+SSnwU2CKlLZzoTTu/E6pyVES4KOLKbs9V4Q?=
 =?us-ascii?Q?ShSUfq5k3JT+jxYA2MrbFaqAoJpSdC0uvzS7EHVsNNSvlFb6nKFaleLfXgfH?=
 =?us-ascii?Q?zaC/QjvpDPSK+gDKKeYjawJGDTMkxm+pJh3aHtSU/kl1j3Gk+p0G1kLUX+wG?=
 =?us-ascii?Q?cs/UgpJPKTFNamC867MT9LGHpP5KWT9ofUSz7m1e/K8PN/q8cw+KgIuwYaUb?=
 =?us-ascii?Q?vfPl8O30VCAGHO+nnHmJdoKx3Q4+JjEx/luqsbHHfn8c7SD+/erBGJKrDokQ?=
 =?us-ascii?Q?cnLWqt4Ay+TlpLOvdIKcCuf+3VyW3t4Qykjn+B/ZPbr+V8CWZez2jIb0jTmT?=
 =?us-ascii?Q?+Eemu7DxMjOuL9jVGOffqdXTvx/+3gICbZ7/ymB+oSuUYuxls/4gqNArnsg3?=
 =?us-ascii?Q?zzhBP/uFTJ1zwgbd03z3yWR0O9Dj9HjX041ntVpOveNZEs7XgDCI6d3rbdvU?=
 =?us-ascii?Q?dkat9XKM0IwJukOeRdn6GqJXy4xxBIIQOJu543HAyqbs5die0agh0cI0SNnP?=
 =?us-ascii?Q?/tI8qlnmdRXQcTzBr5hHdD3Dqx/j/WrHUyO9/8g5TODx/AtZ8zMynESmnpJA?=
 =?us-ascii?Q?yU5XNd5GwUInJyb54yCKnf9sNVUfRcxOmKlEqO9r0XwoWS5//gydgYPPRP3B?=
 =?us-ascii?Q?PD0bQCzWWhtysO8jHwEGiRp+TVRW+NKh3MRohEmaGxbSyDz1IaUercTce3Pq?=
 =?us-ascii?Q?EZvyZZbpEP0AAcxtWo03mr7YuFWyWZGE1IhC2CGGAAOGOrz3enk67v/g/YWv?=
 =?us-ascii?Q?ij8tpowHhJKQkv6K11fo18ytHjtObGsBe2asKxlpRQWHUuFvD4tefs1AWS+O?=
 =?us-ascii?Q?fi1ZVoZE1ORHLR8oQmrdj5WpgfCpfdHKU9CSnDQwU/C/dnKhmggbi8m6MSSf?=
 =?us-ascii?Q?ffNziaFFnlkQTXsaQCSV/Z3+PniHVZdaPNkdb58KZfaebsEsPSHqAkLCeuDF?=
 =?us-ascii?Q?0SGQaCZuWzgnjEt1W708M/pUmCz9ZV4dEfi1APie+am6OWiLu3T2zZ9Khy/2?=
 =?us-ascii?Q?UbfQVuIuijHC4LDhZGqukkegTiU6Hflo2ZROmmGl14xsKw2Q2XfKqQcFArGc?=
 =?us-ascii?Q?6bO6GT7tWIIr5v26l0/kZ8qk6abDf6ODjlVUcZ/p0TNjk73OwOmd1iKlgDaU?=
 =?us-ascii?Q?sNCYVCs1sjHDfmgP6Iue12dqFLGU3FkfABRdWAGWY2CGFMAArflkElv1/9pc?=
 =?us-ascii?Q?XDni1KhsG1Zoy3E9zljYHWjMlRFN1CcMOvETXPp1cPrEDnfl/oLUFpzGX0YW?=
 =?us-ascii?Q?abgX2CdKbHIIqzopOYyE9zz31ZOSnwyl6mqMV8M2OX07dsDDBq4fE8qNHeYd?=
 =?us-ascii?Q?6tt5KDxtm8tHcX9sPfUWegMv7iI/RoNPszSEAUDOkq0AC+2jtybpe5omTFUc?=
 =?us-ascii?Q?eao9ZvuY/DexfQamxkVlxPwf3+DGIUJxvPio71OTQJ1y4aTDqGu4UgQVpFSx?=
 =?us-ascii?Q?SiA5EpFUR433cDtJ4+Lg1DU15GmKVa97/Vv0/H5J0KkoU8j+TY95odag0Mj0?=
 =?us-ascii?Q?8ZD9BtJxmbhPUpCei8S/74mCsi9+3uC5crm49GhudNCtgwjrdV2sBii83/4D?=
 =?us-ascii?Q?Bt84n0GdcKN6L2DQwd40JX4OJZonfCzXdJ55Gn21?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO6PR18MB4419.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 56e7ceb5-69f8-48c3-7be2-08db9740b431
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Aug 2023 12:20:35.9590
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 7aiqzZqVTqbxZx9GUxJAKZfL8Hk4w8TRqiNXffxtIT67hxt/Dlfz3nXGsFf9J4mUR/S/rvqAsAeMPzDNHwaTwg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR18MB4460
X-Proofpoint-GUID: _pvc2BOqQDTuRVA6h9q-eut8UfXzlwui
X-Proofpoint-ORIG-GUID: ASTGHuagPeRGi6EBETMreaOkMGVpnEng
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-08-07_12,2023-08-03_01,2023-05-22_02
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Chengfeng Ye <dg573847474@gmail.com>
> Sent: Wednesday, July 26, 2023 6:27 PM
> To: Nilesh Javali <njavali@marvell.com>; Manish Rangankar
> <mrangankar@marvell.com>; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@marvell.com>; jejb@linux.ibm.com;
> martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Chengfeng Y=
e
> <dg573847474@gmail.com>
> Subject: [EXT] [PATCH] scsi: qedi: Fix potential deadlock on &qedi_percpu=
-
> >p_work_lock
>=20
> External Email
>=20
> ----------------------------------------------------------------------
> 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 i=
rq,
> otherwise deadlock could happen if the irq preempt the execution while th=
e
> lock is held in process context on the same CPU.
>=20
> qedi_cpu_offline() is one such function acquires the lock on process cont=
ext.
>=20
> [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)
>=20
> This flaw was found by an experimental static analysis tool I am developi=
ng
> for irq-related deadlock.
>=20
> The tentative patch fix the potential deadlock by spin_lock_irqsave() und=
er
> process context.
>=20
> Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
> ---
>  drivers/scsi/qedi/qedi_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>=20
> 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 =3D this_cpu_ptr(&qedi_percpu);
>  	struct qedi_work *work, *tmp;
>  	struct task_struct *thread;
> +	unsigned long flags;
>=20
> -	spin_lock_bh(&p->p_work_lock);
> +	spin_lock_irqsave(&p->p_work_lock, flags);
>  	thread =3D p->iothread;
>  	p->iothread =3D NULL;
>=20
> @@ -1988,7 +1989,7 @@ static int qedi_cpu_offline(unsigned int cpu)
>  			kfree(work);
>  	}
>=20
> -	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@marvell.com>