| Message ID | 20220713094548.3958915-1-yukuai1@huaweicloud.com |
|---|---|
| State | New |
| Headers | show |
| Series | [-next,RFC] scsi: ses: fix slab-out-of-bounds in ses_enclosure_data_process | expand |
> After analysis on vmcore, it was found that the line "desc_ptr[len] = > '\0';" has slab-out-of-bounds problem in ses_enclosure_data_process. > In ses_enclosure_data_process, "desc_ptr" point to "buf", so it have > to be limited in the memory of "buf", however. although there is > "desc_ptr >= buf + page7_len" judgment, it does not work because > "desc_ptr + 4 + len" may bigger than "buf + page7_len", which will > lead to slab-out-of-bounds problem. > > Fix it by using judging desc_ptr cross the border or not after > "desc_ptr += 4". FWIW, I tested this change and I am still getting KASAN errors from ses.
Hi
From my description, there is still loophole in the previous changes.
can you make a test with the following changes?
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 0a1734f34587..06b991e27c84 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -559,11 +559,11 @@ static void ses_enclosure_data_process(struct
enclosure_device *edev,
struct enclosure_component *ecomp;
if (desc_ptr) {
- if (desc_ptr >= buf + page7_len) {
+ len = (desc_ptr[2] << 8) + desc_ptr[3];
+ desc_ptr += 4;
+ if (desc_ptr + len > 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';
thanks！
Wensheng
在 2022/8/2 8:01, Martin K. Petersen 写道:
>> After analysis on vmcore, it was found that the line "desc_ptr[len] =
>> '\0';" has slab-out-of-bounds problem in ses_enclosure_data_process.
>> In ses_enclosure_data_process, "desc_ptr" point to "buf", so it have
>> to be limited in the memory of "buf", however. although there is
>> "desc_ptr >= buf + page7_len" judgment, it does not work because
>> "desc_ptr + 4 + len" may bigger than "buf + page7_len", which will
>> lead to slab-out-of-bounds problem.
>>
>> Fix it by using judging desc_ptr cross the border or not after
>> "desc_ptr += 4".
> FWIW, I tested this change and I am still getting KASAN errors from ses.
>
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 0a1734f34587..981e6e950adc 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -559,11 +559,11 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, struct enclosure_component *ecomp; if (desc_ptr) { + len = (desc_ptr[2] << 8) + desc_ptr[3]; + desc_ptr += 4; 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';