From nobody Mon Feb 9 05:22:43 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DD6B1C761A6 for ; Tue, 4 Apr 2023 21:41:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236432AbjDDVlu (ORCPT ); Tue, 4 Apr 2023 17:41:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49156 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234988AbjDDVlt (ORCPT ); Tue, 4 Apr 2023 17:41:49 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C99753AB0; Tue, 4 Apr 2023 14:41:47 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 6456B639B3; Tue, 4 Apr 2023 21:41:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 12877C433D2; Tue, 4 Apr 2023 21:41:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680644506; bh=lMszXUPs3xmT4/L30IdHdQrLlqYnTYyFa1K84xFbLzU=; h=Date:From:To:cc:Subject:In-Reply-To:References:From; b=suGmChBU3viKp/0FZp/PClBH3B/13p89RcgTbkDT3EsMOXC3oDkh6gYjPX2bapCZ/ aGs5TKU6V9bgcqh1+blinlxGO/YS49LgT+ZRsn9CSuZPUp3XhsRjKZLNJQERP6YH7n SH0zxfrw3GYMpps3+hAiPGqPJhzv05asqqtUeUY3Xpu/X0hkpApOyLdZR3MDWmeS9l bznh3G5HdNQLT0FjYySymJB7exOOe9Wsj+D0Gp1bOXwFubBCPhL4NtvoRFPcBS1Tka gIFnl1bh9qp5+7QWCvUEVBHU7dlLZ+y16JCBSFBb75449My+VcOMpvtX1EiA9MhlVW Y9JbxaO1APLFA== Date: Tue, 4 Apr 2023 23:41:42 +0200 (CEST) From: Jiri Kosina To: James Bottomley cc: "Martin K. Petersen" , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, Ding Hui , Michal Kolar Subject: [PATCH v2] scsi: ses: Handle enclosure with just a primary component gracefully In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" On Tue, 4 Apr 2023, James Bottomley wrote: > > This reverts 3fe97ff3d9493 ("scsi: ses: Don't attach if enclosure has > > no components") and introduces proper handling of case where there > > are no detected secondary components, but primary component > > (enumerated in num_enclosures) does exist. That fix was originally > > proposed by Ding Hui . >=20 > I think everything in here looks fine except this: >=20 > > --- a/drivers/scsi/ses.c > > +++ b/drivers/scsi/ses.c > > @@ -509,9 +509,6 @@ static int ses_enclosure_find_by_addr(struct > > enclosure_device *edev, > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int i; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct ses_component *s= comp; > > =C2=A0 > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!edev->component[0].scra= tch) > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0return 0; > > - > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0for (i =3D 0; i < edev-= >components; i++) { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0scomp =3D edev->component[i].scratch; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0if (scomp->addr !=3D efd->addr) >=20 > If you remove the check, then scomp could be NULL here and we'll oops > on scomp->addr. This hunk was taken from the original 2020 fix, but you are right, thanks=20 for spotting this. Please find v2 below, with this hunk removed, and Tested-by: added. From: Jiri Kosina Subject: [PATCH] scsi: ses: Handle enclosure with just a primary component = gracefully This reverts 3fe97ff3d9493 ("scsi: ses: Don't attach if enclosure has no components") and introduces proper handling of case where there are no dete= cted secondary components, but primary component (enumerated in num_enclosures) does exist. That fix was originally proposed by Ding Hui . Completely ignoring devices that have one primary enclosure and no secondar= y one results in ses_intf_add() bailing completely scsi 2:0:0:254: enclosure has no enumerated components scsi 2:0:0:254: Failed to bind enclosure -12ven in valid configurat= ions such even on valid configurations with 1 primary and 0 secondary enclosures as b= elow: # sg_ses /dev/sg0 3PARdata SES 3321 Supported diagnostic pages: Supported Diagnostic Pages [sdp] [0x0] Configuration (SES) [cf] [0x1] Short Enclosure Status (SES) [ses] [0x8] # sg_ses -p cf /dev/sg0 3PARdata SES 3321 Configuration diagnostic page: number of secondary subenclosures: 0 generation code: 0x0 enclosure descriptor list Subenclosure identifier: 0 [primary] relative ES process id: 0, number of ES processes: 1 number of type descriptor headers: 1 enclosure logical identifier (hex): 20000002ac02068d enclosure vendor: 3PARdata product: VV rev: 3321 type descriptor header and text list Element type: Unspecified, subenclosure id: 0 number of possible elements: 1 The changelog for the original fix follows =3D=3D=3D=3D=3D We can get a crash when disconnecting the iSCSI session, the call trace like this: [ffff00002a00fb70] kfree at ffff00000830e224 [ffff00002a00fba0] ses_intf_remove at ffff000001f200e4 [ffff00002a00fbd0] device_del at ffff0000086b6a98 [ffff00002a00fc50] device_unregister at ffff0000086b6d58 [ffff00002a00fc70] __scsi_remove_device at ffff00000870608c [ffff00002a00fca0] scsi_remove_device at ffff000008706134 [ffff00002a00fcc0] __scsi_remove_target at ffff0000087062e4 [ffff00002a00fd10] scsi_remove_target at ffff0000087064c0 [ffff00002a00fd70] __iscsi_unbind_session at ffff000001c872c4 [ffff00002a00fdb0] process_one_work at ffff00000810f35c [ffff00002a00fe00] worker_thread at ffff00000810f648 [ffff00002a00fe70] kthread at ffff000008116e98 In ses_intf_add, components count could be 0, and kcalloc 0 size scomp, but not saved in edev->component[i].scratch In this situation, edev->component[0].scratch is an invalid pointer, when kfree it in ses_intf_remove_enclosure, a crash like above would happen The call trace also could be other random cases when kfree cannot catch the invalid pointer We should not use edev->component[] array when the components count is 0 We also need check index when use edev->component[] array in ses_enclosure_data_process =3D=3D=3D=3D=3D Reported-by: Michal Kolar Tested-by: Michal Kolar Originally-by: Ding Hui Cc: stable@vger.kernel.org Fixes: 3fe97ff3d9493 ("scsi: ses: Don't attach if enclosure has no componen= ts") Signed-off-by: Jiri Kosina --- v1 -> v2: - fix potential oops in ses_enclosure_find_by_addr() spotted by=20 James - add Tested-by: drivers/scsi/ses.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index b11a9162e73a..f3fa92f493ec 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -602,8 +602,10 @@ static void ses_enclosure_data_process(struct enclosur= e_device *edev, components++, type_ptr[0], name); - else + else if (components < edev->components) ecomp =3D &edev->component[components++]; + else + ecomp =3D ERR_PTR(-EINVAL); =20 if (!IS_ERR(ecomp)) { if (addl_desc_ptr) { @@ -734,11 +736,6 @@ static int ses_intf_add(struct device *cdev, components +=3D type_ptr[1]; } =20 - if (components =3D=3D 0) { - sdev_printk(KERN_WARNING, sdev, "enclosure has no enumerated components\= n"); - goto err_free; - } - ses_dev->page1 =3D buf; ses_dev->page1_len =3D len; buf =3D NULL; @@ -780,9 +777,11 @@ static int ses_intf_add(struct device *cdev, buf =3D NULL; } page2_not_supported: - scomp =3D kcalloc(components, sizeof(struct ses_component), GFP_KERNEL); - if (!scomp) - goto err_free; + if (components > 0) { + scomp =3D kcalloc(components, sizeof(struct ses_component), GFP_KERNEL); + if (!scomp) + goto err_free; + } =20 edev =3D enclosure_register(cdev->parent, dev_name(&sdev->sdev_gendev), components, &ses_enclosure_callbacks); --=20 Jiri Kosina SUSE Labs