* [PATCH] scsi: qla2xxx: Fix an integer overflow in sysfs code
[not found] <CAER+mfKPOt03DmPB4FTCjVvQ=2uRSMVbBHP-3CdoG+mQnSiYFw@mail.gmail.com>
@ 2017-08-30 12:21 ` Dan Carpenter
[not found] ` <CAER+mf+sKmuDHmdsx_QfFgA4iNCQkhPAOvJBq3ydKpFXh2hJRQ@mail.gmail.com>
2017-08-30 13:12 ` Greg KH
0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2017-08-30 12:21 UTC (permalink / raw)
To: qla2xxx-upstream, shqking, Joe Carnuccio
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, security
The value of "size" comes from the user. When we add "start + size"
it could lead to an integer overflow bug.
It means we vmalloc() a lot more memory than we had intended. I believe
that on 64 bit systems vmalloc() can succeed even if we ask it to
allocate huge 4GB buffers. So we would get memory corruption and likely
a crash when we call ha->isp_ops->write_optrom() and ->read_optrom().
Only root can trigger this bug.
Fixes: b7cc176c9eb3 ("[SCSI] qla2xxx: Allow region-based flash-part accesses.")
Reported-by: shqking <shqking@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 75c4b312645e..9ce28c4f9812 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -318,6 +318,8 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
return -EINVAL;
if (start > ha->optrom_size)
return -EINVAL;
+ if (size > ha->optrom_size - start)
+ size = ha->optrom_size - start;
mutex_lock(&ha->optrom_mutex);
switch (val) {
@@ -343,8 +345,7 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
}
ha->optrom_region_start = start;
- ha->optrom_region_size = start + size > ha->optrom_size ?
- ha->optrom_size - start : size;
+ ha->optrom_region_size = start + size;
ha->optrom_state = QLA_SREADING;
ha->optrom_buffer = vmalloc(ha->optrom_region_size);
@@ -417,8 +418,7 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
}
ha->optrom_region_start = start;
- ha->optrom_region_size = start + size > ha->optrom_size ?
- ha->optrom_size - start : size;
+ ha->optrom_region_size = start + size;
ha->optrom_state = QLA_SWRITING;
ha->optrom_buffer = vmalloc(ha->optrom_region_size);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: qla2xxx: Fix an integer overflow in sysfs code
[not found] ` <CAER+mf+sKmuDHmdsx_QfFgA4iNCQkhPAOvJBq3ydKpFXh2hJRQ@mail.gmail.com>
@ 2017-08-30 12:36 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2017-08-30 12:36 UTC (permalink / raw)
To: shqking
Cc: qla2xxx-upstream, Joe Carnuccio, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, security
On Wed, Aug 30, 2017 at 08:28:52PM +0800, shqking wrote:
> Hi,
>
> Glad to see it is fixed.
>
> Can I apply for a CVE ID for this bug?
>
We don't handle that on this list. You'd need to ask on
oss-security@lists.openwall.com.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: qla2xxx: Fix an integer overflow in sysfs code
2017-08-30 12:21 ` [PATCH] scsi: qla2xxx: Fix an integer overflow in sysfs code Dan Carpenter
[not found] ` <CAER+mf+sKmuDHmdsx_QfFgA4iNCQkhPAOvJBq3ydKpFXh2hJRQ@mail.gmail.com>
@ 2017-08-30 13:12 ` Greg KH
2017-08-30 13:20 ` Eric Dumazet
1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2017-08-30 13:12 UTC (permalink / raw)
To: Dan Carpenter
Cc: qla2xxx-upstream, shqking, Joe Carnuccio, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, security
On Wed, Aug 30, 2017 at 03:21:07PM +0300, Dan Carpenter wrote:
> The value of "size" comes from the user. When we add "start + size"
> it could lead to an integer overflow bug.
>
> It means we vmalloc() a lot more memory than we had intended. I believe
> that on 64 bit systems vmalloc() can succeed even if we ask it to
> allocate huge 4GB buffers. So we would get memory corruption and likely
> a crash when we call ha->isp_ops->write_optrom() and ->read_optrom().
>
> Only root can trigger this bug.
>
> Fixes: b7cc176c9eb3 ("[SCSI] qla2xxx: Allow region-based flash-part accesses.")
> Reported-by: shqking <shqking@gmail.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: stable <stable@vger.kernel.org>
perhaps?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: qla2xxx: Fix an integer overflow in sysfs code
2017-08-30 13:12 ` Greg KH
@ 2017-08-30 13:20 ` Eric Dumazet
2017-08-30 13:30 ` [PATCH v2] " Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2017-08-30 13:20 UTC (permalink / raw)
To: Greg KH
Cc: Dan Carpenter, qla2xxx-upstream, shqking, Joe Carnuccio,
James E.J. Bottomley, Martin K. Petersen, linux-scsi, security
On Wed, Aug 30, 2017 at 6:12 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Aug 30, 2017 at 03:21:07PM +0300, Dan Carpenter wrote:
>> The value of "size" comes from the user. When we add "start + size"
>> it could lead to an integer overflow bug.
>>
>> It means we vmalloc() a lot more memory than we had intended. I believe
>> that on 64 bit systems vmalloc() can succeed even if we ask it to
>> allocate huge 4GB buffers. So we would get memory corruption and likely
>> a crash when we call ha->isp_ops->write_optrom() and ->read_optrom().
>>
>> Only root can trigger this bug.
>>
>> Fixes: b7cc176c9eb3 ("[SCSI] qla2xxx: Allow region-based flash-part accesses.")
>> Reported-by: shqking <shqking@gmail.com>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
>
> Cc: stable <stable@vger.kernel.org>
>
> perhaps?
>
> thanks,
>
> greg k-h
Also a link to https://bugzilla.kernel.org/show_bug.cgi?id=194061
since shqking did a fair bit of analysis, not only report the bug.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] scsi: qla2xxx: Fix an integer overflow in sysfs code
2017-08-30 13:20 ` Eric Dumazet
@ 2017-08-30 13:30 ` Dan Carpenter
2017-08-30 17:08 ` Carnuccio, Joe
2017-08-31 2:07 ` Martin K. Petersen
0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2017-08-30 13:30 UTC (permalink / raw)
To: qla2xxx-upstream, shqking, Joe Carnuccio
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, security
The value of "size" comes from the user. When we add "start + size"
it could lead to an integer overflow bug.
It means we vmalloc() a lot more memory than we had intended. I believe
that on 64 bit systems vmalloc() can succeed even if we ask it to
allocate huge 4GB buffers. So we would get memory corruption and likely
a crash when we call ha->isp_ops->write_optrom() and ->read_optrom().
Only root can trigger this bug.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=194061
Cc: stable@vger.kernel.org
Fixes: b7cc176c9eb3 ("[SCSI] qla2xxx: Allow region-based flash-part accesses.")
Reported-by: shqking <shqking@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Add stable and the URL for bugzila
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 75c4b312645e..9ce28c4f9812 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -318,6 +318,8 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
return -EINVAL;
if (start > ha->optrom_size)
return -EINVAL;
+ if (size > ha->optrom_size - start)
+ size = ha->optrom_size - start;
mutex_lock(&ha->optrom_mutex);
switch (val) {
@@ -343,8 +345,7 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
}
ha->optrom_region_start = start;
- ha->optrom_region_size = start + size > ha->optrom_size ?
- ha->optrom_size - start : size;
+ ha->optrom_region_size = start + size;
ha->optrom_state = QLA_SREADING;
ha->optrom_buffer = vmalloc(ha->optrom_region_size);
@@ -417,8 +418,7 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
}
ha->optrom_region_start = start;
- ha->optrom_region_size = start + size > ha->optrom_size ?
- ha->optrom_size - start : size;
+ ha->optrom_region_size = start + size;
ha->optrom_state = QLA_SWRITING;
ha->optrom_buffer = vmalloc(ha->optrom_region_size);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH v2] scsi: qla2xxx: Fix an integer overflow in sysfs code
2017-08-30 13:30 ` [PATCH v2] " Dan Carpenter
@ 2017-08-30 17:08 ` Carnuccio, Joe
2017-08-31 2:07 ` Martin K. Petersen
1 sibling, 0 replies; 7+ messages in thread
From: Carnuccio, Joe @ 2017-08-30 17:08 UTC (permalink / raw)
To: Dan Carpenter, Dept-Eng QLA2xxx Upstream, shqking
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, security
good catch, thanks; I'll propagate this to our various OOB drivers.
-----Original Message-----
From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
Sent: Wednesday, August 30, 2017 6:31 AM
To: Dept-Eng QLA2xxx Upstream <qla2xxx-upstream@cavium.com>; shqking <shqking@gmail.com>; Carnuccio, Joe <Joe.Carnuccio@cavium.com>
Cc: James E.J. Bottomley <jejb@linux.vnet.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; linux-scsi@vger.kernel.org; security@kernel.org
Subject: [PATCH v2] scsi: qla2xxx: Fix an integer overflow in sysfs code
The value of "size" comes from the user. When we add "start + size"
it could lead to an integer overflow bug.
It means we vmalloc() a lot more memory than we had intended. I believe that on 64 bit systems vmalloc() can succeed even if we ask it to allocate huge 4GB buffers. So we would get memory corruption and likely a crash when we call ha->isp_ops->write_optrom() and ->read_optrom().
Only root can trigger this bug.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=194061
Cc: stable@vger.kernel.org
Fixes: b7cc176c9eb3 ("[SCSI] qla2xxx: Allow region-based flash-part accesses.")
Reported-by: shqking <shqking@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Add stable and the URL for bugzila
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c index 75c4b312645e..9ce28c4f9812 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -318,6 +318,8 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
return -EINVAL;
if (start > ha->optrom_size)
return -EINVAL;
+ if (size > ha->optrom_size - start)
+ size = ha->optrom_size - start;
mutex_lock(&ha->optrom_mutex);
switch (val) {
@@ -343,8 +345,7 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
}
ha->optrom_region_start = start;
- ha->optrom_region_size = start + size > ha->optrom_size ?
- ha->optrom_size - start : size;
+ ha->optrom_region_size = start + size;
ha->optrom_state = QLA_SREADING;
ha->optrom_buffer = vmalloc(ha->optrom_region_size); @@ -417,8 +418,7 @@ qla2x00_sysfs_write_optrom_ctl(struct file *filp, struct kobject *kobj,
}
ha->optrom_region_start = start;
- ha->optrom_region_size = start + size > ha->optrom_size ?
- ha->optrom_size - start : size;
+ ha->optrom_region_size = start + size;
ha->optrom_state = QLA_SWRITING;
ha->optrom_buffer = vmalloc(ha->optrom_region_size);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] scsi: qla2xxx: Fix an integer overflow in sysfs code
2017-08-30 13:30 ` [PATCH v2] " Dan Carpenter
2017-08-30 17:08 ` Carnuccio, Joe
@ 2017-08-31 2:07 ` Martin K. Petersen
1 sibling, 0 replies; 7+ messages in thread
From: Martin K. Petersen @ 2017-08-31 2:07 UTC (permalink / raw)
To: Dan Carpenter
Cc: qla2xxx-upstream, shqking, Joe Carnuccio, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, security
Dan,
> The value of "size" comes from the user. When we add "start + size"
> it could lead to an integer overflow bug.
>
> It means we vmalloc() a lot more memory than we had intended. I
> believe that on 64 bit systems vmalloc() can succeed even if we ask it
> to allocate huge 4GB buffers. So we would get memory corruption and
> likely a crash when we call ha->isp_ops->write_optrom() and
> ->read_optrom().
Applied to 4.13/scsi-fixes. Thank you!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-08-31 2:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAER+mfKPOt03DmPB4FTCjVvQ=2uRSMVbBHP-3CdoG+mQnSiYFw@mail.gmail.com>
2017-08-30 12:21 ` [PATCH] scsi: qla2xxx: Fix an integer overflow in sysfs code Dan Carpenter
[not found] ` <CAER+mf+sKmuDHmdsx_QfFgA4iNCQkhPAOvJBq3ydKpFXh2hJRQ@mail.gmail.com>
2017-08-30 12:36 ` Dan Carpenter
2017-08-30 13:12 ` Greg KH
2017-08-30 13:20 ` Eric Dumazet
2017-08-30 13:30 ` [PATCH v2] " Dan Carpenter
2017-08-30 17:08 ` Carnuccio, Joe
2017-08-31 2:07 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.