| Message ID | nycvar.YFH.7.76.2304042122270.29760@cbobk.fhfr.pm |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ses: Handle enclosure with just a primary component gracefully | expand |
On 2023/4/5 4:26, James Bottomley wrote: > On Tue, 2023-04-04 at 21:23 +0200, Jiri Kosina wrote: >> From: Jiri Kosina <jkosina@suse.cz> >> >> 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 <dinghui@sangfor.com.cn>. > > I think everything in here looks fine except this: > >> --- 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, >> int i; >> struct ses_component *scomp; >> >> - if (!edev->component[0].scratch) >> - return 0; >> - >> for (i = 0; i < edev->components; i++) { >> scomp = edev->component[i].scratch; >> if (scomp->addr != efd->addr) > > If you remove the check, then scomp could be NULL here and we'll oops > on scomp->addr. I think we should remove the check, because the edev->components represented the effectiveness of array pointers, so we need check edev->components firstly instead of checking edev->component[0].scratch, if edev->components is 0, we won't enter the for loop, don't worry about dereference scomp. > > Regards, > > James > >
On Tue, 04 Apr 2023 21:23:42 +0200, Jiri Kosina 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 <dinghui@sangfor.com.cn>. > > Completely ignoring devices that have one primary enclosure and no secondary one > results in ses_intf_add() bailing completely > > [...] Applied to 6.3/scsi-fixes, thanks! [1/1] scsi: ses: Handle enclosure with just a primary component gracefully https://git.kernel.org/mkp/scsi/c/c8e22b7a1694
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index b11a9162e73a..b54f2c6c08c3 100644 --- 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, int i; struct ses_component *scomp; - if (!edev->component[0].scratch) - return 0; - for (i = 0; i < edev->components; i++) { scomp = edev->component[i].scratch; if (scomp->addr != efd->addr) @@ -602,8 +599,10 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, components++, type_ptr[0], name); - else + else if (components < edev->components) ecomp = &edev->component[components++]; + else + ecomp = ERR_PTR(-EINVAL); if (!IS_ERR(ecomp)) { if (addl_desc_ptr) { @@ -734,11 +733,6 @@ static int ses_intf_add(struct device *cdev, 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_len = len; buf = NULL; @@ -780,9 +774,11 @@ static int ses_intf_add(struct device *cdev, buf = NULL; } page2_not_supported: - scomp = kcalloc(components, sizeof(struct ses_component), GFP_KERNEL); - if (!scomp) - goto err_free; + if (components > 0) { + scomp = kcalloc(components, sizeof(struct ses_component), GFP_KERNEL); + if (!scomp) + goto err_free; + } edev = enclosure_register(cdev->parent, dev_name(&sdev->sdev_gendev), components, &ses_enclosure_callbacks);