Skip to content

Commit c8e22b7

Browse files
Jiri Kosinamartinkpetersen
authored andcommitted
scsi: ses: Handle enclosure with just a primary component gracefully
This reverts commit 3fe97ff ("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 <dinghui@sangfor.com.cn>. Completely ignoring devices that have one primary enclosure and no secondary 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 configurations such even on valid configurations with 1 primary and 0 secondary enclosures as below: # 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 ===== 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 ===== Reported-by: Michal Kolar <mich.k@seznam.cz> Originally-by: Ding Hui <dinghui@sangfor.com.cn> Cc: stable@vger.kernel.org Fixes: 3fe97ff ("scsi: ses: Don't attach if enclosure has no components") Signed-off-by: Jiri Kosina <jkosina@suse.cz> Link: https://lore.kernel.org/r/nycvar.YFH.7.76.2304042122270.29760@cbobk.fhfr.pm Tested-by: Michal Kolar <mich.k@seznam.cz> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 48b19b7 commit c8e22b7

1 file changed

Lines changed: 8 additions & 12 deletions

File tree

drivers/scsi/ses.c

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,9 +509,6 @@ static int ses_enclosure_find_by_addr(struct enclosure_device *edev,
509509
int i;
510510
struct ses_component *scomp;
511511

512-
if (!edev->component[0].scratch)
513-
return 0;
514-
515512
for (i = 0; i < edev->components; i++) {
516513
scomp = edev->component[i].scratch;
517514
if (scomp->addr != efd->addr)
@@ -602,8 +599,10 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
602599
components++,
603600
type_ptr[0],
604601
name);
605-
else
602+
else if (components < edev->components)
606603
ecomp = &edev->component[components++];
604+
else
605+
ecomp = ERR_PTR(-EINVAL);
607606

608607
if (!IS_ERR(ecomp)) {
609608
if (addl_desc_ptr) {
@@ -734,11 +733,6 @@ static int ses_intf_add(struct device *cdev,
734733
components += type_ptr[1];
735734
}
736735

737-
if (components == 0) {
738-
sdev_printk(KERN_WARNING, sdev, "enclosure has no enumerated components\n");
739-
goto err_free;
740-
}
741-
742736
ses_dev->page1 = buf;
743737
ses_dev->page1_len = len;
744738
buf = NULL;
@@ -780,9 +774,11 @@ static int ses_intf_add(struct device *cdev,
780774
buf = NULL;
781775
}
782776
page2_not_supported:
783-
scomp = kcalloc(components, sizeof(struct ses_component), GFP_KERNEL);
784-
if (!scomp)
785-
goto err_free;
777+
if (components > 0) {
778+
scomp = kcalloc(components, sizeof(struct ses_component), GFP_KERNEL);
779+
if (!scomp)
780+
goto err_free;
781+
}
786782

787783
edev = enclosure_register(cdev->parent, dev_name(&sdev->sdev_gendev),
788784
components, &ses_enclosure_callbacks);

0 commit comments

Comments
 (0)