* [PATCH v2 1/4] ses: fix slab-out-of-bounds reported by KASAN in ses_enclosure_data_process
2023-02-02 16:24 [PATCH v2 0/4] ses: prevent from out of bounds accesses Tomas Henzl
@ 2023-02-02 16:24 ` Tomas Henzl
2023-02-02 16:24 ` [PATCH v2 2/4] ses: fix possible addl_desc_ptr out-of-bounds accesses " Tomas Henzl
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Tomas Henzl @ 2023-02-02 16:24 UTC (permalink / raw)
To: linux-scsi; +Cc: mikoxyzzz
A fix for:
BUG: KASAN: slab-out-of-bounds in ses_enclosure_data_process+0x949/0xe30 [ses]
Read of size 1 at addr ffff88a1b043a451 by task systemd-udevd/3271
Checking after (and before in next loop) addl_desc_ptr[1] is sufficient,
we expect the size to be sanitized before first access to addl_desc_ptr[1].
Testing for one more byte shall protect partially the ses_process_descriptor.
Signed-off-by: Tomas Henzl <thenzl@redhat.com>
---
drivers/scsi/ses.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 0a1734f34587..1ee927ac8603 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -603,9 +603,11 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
/* these elements are optional */
type_ptr[0] == ENCLOSURE_COMPONENT_SCSI_TARGET_PORT ||
type_ptr[0] == ENCLOSURE_COMPONENT_SCSI_INITIATOR_PORT ||
- type_ptr[0] == ENCLOSURE_COMPONENT_CONTROLLER_ELECTRONICS))
+ type_ptr[0] == ENCLOSURE_COMPONENT_CONTROLLER_ELECTRONICS)){
addl_desc_ptr += addl_desc_ptr[1] + 2;
-
+ if (addl_desc_ptr + 1 >= ses_dev->page10 + ses_dev->page10_len)
+ addl_desc_ptr = NULL;
+ }
}
}
kfree(buf);
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/4] ses: fix possible addl_desc_ptr out-of-bounds accesses in ses_enclosure_data_process
2023-02-02 16:24 [PATCH v2 0/4] ses: prevent from out of bounds accesses Tomas Henzl
2023-02-02 16:24 ` [PATCH v2 1/4] ses: fix slab-out-of-bounds reported by KASAN in ses_enclosure_data_process Tomas Henzl
@ 2023-02-02 16:24 ` Tomas Henzl
2023-02-02 16:24 ` [PATCH v2 3/4] ses: fix possible desc_ptr " Tomas Henzl
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Tomas Henzl @ 2023-02-02 16:24 UTC (permalink / raw)
To: linux-scsi; +Cc: mikoxyzzz
Sanitize possible addl_desc_ptr out-of-bounds accesses
in ses_enclosure_data_process.
Signed-off-by: Tomas Henzl <thenzl@redhat.com>
---
drivers/scsi/ses.c | 35 ++++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 1ee927ac8603..896fd4f6e93d 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -433,8 +433,8 @@ int ses_match_host(struct enclosure_device *edev, void *data)
}
#endif /* 0 */
-static void ses_process_descriptor(struct enclosure_component *ecomp,
- unsigned char *desc)
+static int ses_process_descriptor(struct enclosure_component *ecomp,
+ unsigned char *desc, int max_desc_len)
{
int eip = desc[0] & 0x10;
int invalid = desc[0] & 0x80;
@@ -445,22 +445,32 @@ static void ses_process_descriptor(struct enclosure_component *ecomp,
unsigned char *d;
if (invalid)
- return;
+ return 0;
switch (proto) {
case SCSI_PROTOCOL_FCP:
if (eip) {
+ if (max_desc_len <= 7)
+ return 1;
d = desc + 4;
slot = d[3];
}
break;
case SCSI_PROTOCOL_SAS:
+
if (eip) {
+ if (max_desc_len <= 27)
+ return 1;
d = desc + 4;
slot = d[3];
d = desc + 8;
- } else
+ } else {
+ if (max_desc_len <= 23)
+ return 1;
d = desc + 4;
+ }
+
+
/* only take the phy0 addr */
addr = (u64)d[12] << 56 |
(u64)d[13] << 48 |
@@ -477,6 +487,8 @@ static void ses_process_descriptor(struct enclosure_component *ecomp,
}
ecomp->slot = slot;
scomp->addr = addr;
+
+ return 0;
}
struct efd {
@@ -549,7 +561,7 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
/* skip past overall descriptor */
desc_ptr += len + 4;
}
- if (ses_dev->page10)
+ if (ses_dev->page10 && ses_dev->page10_len > 9)
addl_desc_ptr = ses_dev->page10 + 8;
type_ptr = ses_dev->page1_types;
components = 0;
@@ -557,6 +569,7 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
for (j = 0; j < type_ptr[1]; j++) {
char *name = NULL;
struct enclosure_component *ecomp;
+ int max_desc_len;
if (desc_ptr) {
if (desc_ptr >= buf + page7_len) {
@@ -583,10 +596,14 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
ecomp = &edev->component[components++];
if (!IS_ERR(ecomp)) {
- if (addl_desc_ptr)
- ses_process_descriptor(
- ecomp,
- addl_desc_ptr);
+ if (addl_desc_ptr) {
+ max_desc_len = ses_dev->page10_len -
+ (addl_desc_ptr - ses_dev->page10);
+ if (ses_process_descriptor(ecomp,
+ addl_desc_ptr,
+ max_desc_len))
+ addl_desc_ptr = NULL;
+ }
if (create)
enclosure_component_register(
ecomp);
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/4] ses: fix possible desc_ptr out-of-bounds accesses in ses_enclosure_data_process
2023-02-02 16:24 [PATCH v2 0/4] ses: prevent from out of bounds accesses Tomas Henzl
2023-02-02 16:24 ` [PATCH v2 1/4] ses: fix slab-out-of-bounds reported by KASAN in ses_enclosure_data_process Tomas Henzl
2023-02-02 16:24 ` [PATCH v2 2/4] ses: fix possible addl_desc_ptr out-of-bounds accesses " Tomas Henzl
@ 2023-02-02 16:24 ` Tomas Henzl
2023-02-02 16:24 ` [PATCH v2 4/4] ses: fix slab-out-of-bounds reported by KASAN in ses_intf_remove Tomas Henzl
2023-02-21 22:59 ` [PATCH v2 0/4] ses: prevent from out of bounds accesses Martin K. Petersen
4 siblings, 0 replies; 7+ messages in thread
From: Tomas Henzl @ 2023-02-02 16:24 UTC (permalink / raw)
To: linux-scsi; +Cc: mikoxyzzz
Sanitize possible desc_ptr out-of-bounds accesses
in ses_enclosure_data_process.
Signed-off-by: Tomas Henzl <thenzl@redhat.com>
---
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 896fd4f6e93d..dbfe12f63c98 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -572,15 +572,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 ||
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 4/4] ses: fix slab-out-of-bounds reported by KASAN in ses_intf_remove
2023-02-02 16:24 [PATCH v2 0/4] ses: prevent from out of bounds accesses Tomas Henzl
` (2 preceding siblings ...)
2023-02-02 16:24 ` [PATCH v2 3/4] ses: fix possible desc_ptr " Tomas Henzl
@ 2023-02-02 16:24 ` Tomas Henzl
2023-02-21 22:59 ` [PATCH v2 0/4] ses: prevent from out of bounds accesses Martin K. Petersen
4 siblings, 0 replies; 7+ messages in thread
From: Tomas Henzl @ 2023-02-02 16:24 UTC (permalink / raw)
To: linux-scsi; +Cc: mikoxyzzz
A fix for:
BUG: KASAN: slab-out-of-bounds in ses_intf_remove+0x23f/0x270 [ses]
Read of size 8 at addr ffff88a10d32e5d8 by task rmmod/12013
When edev->components is zero, accessing edev->component[0]
members is wrong.
Signed-off-by: Tomas Henzl <thenzl@redhat.com>
---
drivers/scsi/ses.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index dbfe12f63c98..7a9eb54e8808 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -850,7 +850,8 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev)
kfree(ses_dev->page2);
kfree(ses_dev);
- kfree(edev->component[0].scratch);
+ if (edev->components)
+ kfree(edev->component[0].scratch);
put_device(&edev->edev);
enclosure_unregister(edev);
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/4] ses: prevent from out of bounds accesses
2023-02-02 16:24 [PATCH v2 0/4] ses: prevent from out of bounds accesses Tomas Henzl
` (3 preceding siblings ...)
2023-02-02 16:24 ` [PATCH v2 4/4] ses: fix slab-out-of-bounds reported by KASAN in ses_intf_remove Tomas Henzl
@ 2023-02-21 22:59 ` Martin K. Petersen
2023-02-22 15:24 ` Tomas Henzl
4 siblings, 1 reply; 7+ messages in thread
From: Martin K. Petersen @ 2023-02-21 22:59 UTC (permalink / raw)
To: Tomas Henzl; +Cc: linux-scsi, mikoxyzzz
Tomas,
> First patch fixes a KASAN reported problem Second patch fixes other
> possible places in ses_enclosure_data_process where the max_desc_len
> might access memory out of bounds. 3/4 does the same for desc_ptr in
> ses_enclosure_data_process. The last patch fixes another KASAN report
> in ses_intf_remove.
Thanks for working on this! With your series applied, in combination
with a straggling patch from James, I can finally boot my SAS test setup
without any KASAN warnings.
Applied to 6.3/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/4] ses: prevent from out of bounds accesses
2023-02-21 22:59 ` [PATCH v2 0/4] ses: prevent from out of bounds accesses Martin K. Petersen
@ 2023-02-22 15:24 ` Tomas Henzl
0 siblings, 0 replies; 7+ messages in thread
From: Tomas Henzl @ 2023-02-22 15:24 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: linux-scsi, mikoxyzzz
On 2/21/23 23:59, Martin K. Petersen wrote:
>
> Tomas,
>
>> First patch fixes a KASAN reported problem Second patch fixes other
>> possible places in ses_enclosure_data_process where the max_desc_len
>> might access memory out of bounds. 3/4 does the same for desc_ptr in
>> ses_enclosure_data_process. The last patch fixes another KASAN report
>> in ses_intf_remove.
>
> Thanks for working on this! With your series applied, in combination
> with a straggling patch from James, I can finally boot my SAS test setup
> without any KASAN warnings.
I'm glad it worked for you and it could have been added since I've
noticed some previous approaches rejected.
>
> Applied to 6.3/scsi-staging, thanks!
>
^ permalink raw reply [flat|nested] 7+ messages in thread