From 9530f1fcea67b10ea64db0e783488bab0ebf1f47 Mon Sep 17 00:00:00 2001 From: Li Lingfeng Date: Mon, 29 Apr 2024 09:49:18 +0800 Subject: [PATCH 1/5] Revert "scsi: ses: fix slab-out-of-bounds in ses_enclosure_data_process" hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I9K8D1 CVE: NA -------------------------------- This reverts commit d77d7bbc1c59243c536a82076c8a3bdc87b98e41. Backport mainline patches(584892fd29a4 "scsi: ses: Fix possible desc_ptr out-of-bounds accesses") to replace it. Fixes: d77d7bbc1c59 ("scsi: ses: fix slab-out-of-bounds in ses_enclosure_data_process") Signed-off-by: Li Lingfeng --- drivers/scsi/ses.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 6b0397e5d47b..d6a8f7c0192c 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -569,11 +569,11 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, int max_desc_len; if (desc_ptr) { - len = (desc_ptr[2] << 8) + desc_ptr[3]; - desc_ptr += 4; - if (desc_ptr + len > buf + page7_len) { + if (desc_ptr >= buf + page7_len) { desc_ptr = NULL; } else { + len = (desc_ptr[2] << 8) + desc_ptr[3]; + desc_ptr += 4; /* Add trailing zero - pushes into * reserved space */ desc_ptr[len] = '\0'; -- Gitee From 9a5c6ac0377630b9a15af50f5d0e5fee6be3d914 Mon Sep 17 00:00:00 2001 From: Tomas Henzl Date: Mon, 29 Apr 2024 09:49:19 +0800 Subject: [PATCH 2/5] scsi: ses: Fix possible desc_ptr out-of-bounds accesses stable inclusion from stable-v5.10.173 commit c315560e3ef77c1d822249f1743e647dc9c9912a category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I9K8D1 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.10.191&id=c315560e3ef77c1d822249f1743e647dc9c9912a ---------------------------------------- commit 801ab13d50cf3d26170ee073ea8bb4eececb76ab upstream. Sanitize possible desc_ptr out-of-bounds accesses in ses_enclosure_data_process(). Link: https://lore.kernel.org/r/20230202162451.15346-4-thenzl@redhat.com Cc: stable@vger.kernel.org Signed-off-by: Tomas Henzl Signed-off-by: Martin K. Petersen Signed-off-by: Greg Kroah-Hartman Signed-off-by: Li Lingfeng --- drivers/scsi/ses.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index d6a8f7c0192c..14dd4462ea70 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -569,15 +569,19 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, int max_desc_len; if (desc_ptr) { - if (desc_ptr >= buf + page7_len) { + if (desc_ptr + 3 >= buf + page7_len) { desc_ptr = NULL; } else { len = (desc_ptr[2] << 8) + desc_ptr[3]; desc_ptr += 4; - /* Add trailing zero - pushes into - * reserved space */ - desc_ptr[len] = '\0'; - name = desc_ptr; + if (desc_ptr + len > buf + page7_len) + desc_ptr = NULL; + else { + /* Add trailing zero - pushes into + * reserved space */ + desc_ptr[len] = '\0'; + name = desc_ptr; + } } } if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE || -- Gitee From 3f7df9882a7a20d3250277de621f612b58a5c5a1 Mon Sep 17 00:00:00 2001 From: Li Lingfeng Date: Mon, 29 Apr 2024 09:49:20 +0800 Subject: [PATCH 3/5] Revert "scsi: ses: Fix crash caused by kfree an invalid pointer" hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I9K8D1 CVE: NA -------------------------------- This reverts commit 74885845d10e5d9ab601678b610a76fd1012df92. Backport 176d7345b89c("scsi: ses: Handle enclosure with just a primary component gracefully") to replace it. Fixes: 74885845d10e ("scsi: ses: Fix crash caused by kfree an invalid pointer") Signed-off-by: Li Lingfeng --- drivers/scsi/ses.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 14dd4462ea70..6b50d9cdb149 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -503,6 +503,9 @@ 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) @@ -593,10 +596,8 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, components++, type_ptr[0], name); - else if (components < edev->components) - ecomp = &edev->component[components++]; else - ecomp = ERR_PTR(-EINVAL); + ecomp = &edev->component[components++]; if (!IS_ERR(ecomp)) { if (addl_desc_ptr) { @@ -768,11 +769,9 @@ static int ses_intf_add(struct device *cdev, buf = NULL; } page2_not_supported: - if (components > 0) { - scomp = kcalloc(components, sizeof(struct ses_component), GFP_KERNEL); - if (!scomp) - goto err_free; - } + 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); @@ -852,8 +851,7 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev) kfree(ses_dev->page2); kfree(ses_dev); - if (edev->components > 0) - kfree(edev->component[0].scratch); + kfree(edev->component[0].scratch); put_device(&edev->edev); enclosure_unregister(edev); -- Gitee From a6016309ac3dae9421d3641e2af05dc90d38e556 Mon Sep 17 00:00:00 2001 From: Li Lingfeng Date: Mon, 29 Apr 2024 09:49:21 +0800 Subject: [PATCH 4/5] Revert "scsi: ses: Handle enclosure with just a primary component gracefully" hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I9K8D1 CVE: NA -------------------------------- This reverts commit b9b8b793401cab3c8866bb73b2ce0045458e43af. Commit b9b8b793401c ("scsi: ses: Handle enclosure with just a primary component gracefully") tried to apply commit 176d7345b89c from stable. However, it lost most of the changes without any descriptions. Revert it and reapply the patch from stable later. Fixes: b9b8b793401c ("scsi: ses: Handle enclosure with just a primary component gracefully") Signed-off-by: Li Lingfeng --- drivers/scsi/ses.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 6b50d9cdb149..77f4322e2f71 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -728,6 +728,11 @@ 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; -- Gitee From 0dc7c462d7f25cfe5e293eb94a8645ed2abaeb08 Mon Sep 17 00:00:00 2001 From: Jiri Kosina Date: Mon, 29 Apr 2024 09:49:22 +0800 Subject: [PATCH 5/5] scsi: ses: Handle enclosure with just a primary component gracefully stable inclusion from stable-v5.10.178 commit 176d7345b89ced72020a313bfa4e7f345d1c3aed category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I9K8D1 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.10.191&id=176d7345b89ced72020a313bfa4e7f345d1c3aed ---------------------------------------- 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 . 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 Originally-by: Ding Hui Cc: stable@vger.kernel.org Fixes: 3fe97ff3d949 ("scsi: ses: Don't attach if enclosure has no components") Signed-off-by: Jiri Kosina Link: https://lore.kernel.org/r/nycvar.YFH.7.76.2304042122270.29760@cbobk.fhfr.pm Tested-by: Michal Kolar Signed-off-by: Martin K. Petersen Signed-off-by: Greg Kroah-Hartman Signed-off-by: Li Lingfeng --- drivers/scsi/ses.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 77f4322e2f71..942543b34c13 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -503,9 +503,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) @@ -596,8 +593,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) { @@ -728,11 +727,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; @@ -774,9 +768,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); -- Gitee