scsi: ses: Handle enclosure with just a primary component gracefully
commit c8e22b7a1694bb8d025ea636816472739d859145 upstream.
This reverts commit 3fe97ff3d949 ("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: 3fe97ff3d949 ("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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
cf22c98bcb
commit
176d7345b8
@@ -503,9 +503,6 @@ static int ses_enclosure_find_by_addr(struct enclosure_device *edev,
|
|||||||
int i;
|
int i;
|
||||||
struct ses_component *scomp;
|
struct ses_component *scomp;
|
||||||
|
|
||||||
if (!edev->component[0].scratch)
|
|
||||||
return 0;
|
|
||||||
|
|
||||||
for (i = 0; i < edev->components; i++) {
|
for (i = 0; i < edev->components; i++) {
|
||||||
scomp = edev->component[i].scratch;
|
scomp = edev->component[i].scratch;
|
||||||
if (scomp->addr != efd->addr)
|
if (scomp->addr != efd->addr)
|
||||||
@@ -596,8 +593,10 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
|
|||||||
components++,
|
components++,
|
||||||
type_ptr[0],
|
type_ptr[0],
|
||||||
name);
|
name);
|
||||||
else
|
else if (components < edev->components)
|
||||||
ecomp = &edev->component[components++];
|
ecomp = &edev->component[components++];
|
||||||
|
else
|
||||||
|
ecomp = ERR_PTR(-EINVAL);
|
||||||
|
|
||||||
if (!IS_ERR(ecomp)) {
|
if (!IS_ERR(ecomp)) {
|
||||||
if (addl_desc_ptr) {
|
if (addl_desc_ptr) {
|
||||||
@@ -728,11 +727,6 @@ static int ses_intf_add(struct device *cdev,
|
|||||||
components += type_ptr[1];
|
components += type_ptr[1];
|
||||||
}
|
}
|
||||||
|
|
||||||
if (components == 0) {
|
|
||||||
sdev_printk(KERN_WARNING, sdev, "enclosure has no enumerated components\n");
|
|
||||||
goto err_free;
|
|
||||||
}
|
|
||||||
|
|
||||||
ses_dev->page1 = buf;
|
ses_dev->page1 = buf;
|
||||||
ses_dev->page1_len = len;
|
ses_dev->page1_len = len;
|
||||||
buf = NULL;
|
buf = NULL;
|
||||||
@@ -774,9 +768,11 @@ static int ses_intf_add(struct device *cdev,
|
|||||||
buf = NULL;
|
buf = NULL;
|
||||||
}
|
}
|
||||||
page2_not_supported:
|
page2_not_supported:
|
||||||
scomp = kcalloc(components, sizeof(struct ses_component), GFP_KERNEL);
|
if (components > 0) {
|
||||||
if (!scomp)
|
scomp = kcalloc(components, sizeof(struct ses_component), GFP_KERNEL);
|
||||||
goto err_free;
|
if (!scomp)
|
||||||
|
goto err_free;
|
||||||
|
}
|
||||||
|
|
||||||
edev = enclosure_register(cdev->parent, dev_name(&sdev->sdev_gendev),
|
edev = enclosure_register(cdev->parent, dev_name(&sdev->sdev_gendev),
|
||||||
components, &ses_enclosure_callbacks);
|
components, &ses_enclosure_callbacks);
|
||||||
|
|||||||
Reference in New Issue
Block a user