Commit 2a60965a authored by Milan P. Gandhi's avatar Milan P. Gandhi Committed by Greg Kroah-Hartman

scsi: qla2xxx: Get mutex lock before checking optrom_state


[ Upstream commit c7702b8c ]

There is a race condition with qla2xxx optrom functions where one thread
might modify optrom buffer, optrom_state while other thread is still
reading from it.

In couple of crashes, it was found that we had successfully passed the
following 'if' check where we confirm optrom_state to be
QLA_SREADING. But by the time we acquired mutex lock to proceed with
memory_read_from_buffer function, some other thread/process had already
modified that option rom buffer and optrom_state from QLA_SREADING to
QLA_SWAITING. Then we got ha->optrom_buffer 0x0 and crashed the system:

        if (ha->optrom_state != QLA_SREADING)
                return 0;

        mutex_lock(&ha->optrom_mutex);
        rval = memory_read_from_buffer(buf, count, &off, ha->optrom_buffer,
            ha->optrom_region_size);
        mutex_unlock(&ha->optrom_mutex);

With current optrom function we get following crash due to a race
condition:

[ 1479.466679] BUG: unable to handle kernel NULL pointer dereference at           (null)
[ 1479.466707] IP: [<ffffffff81326756>] memcpy+0x6/0x110
[...]
[ 1479.473673] Call Trace:
[ 1479.474296]  [<ffffffff81225cbc>] ? memory_read_from_buffer+0x3c/0x60
[ 1479.474941]  [<ffffffffa01574dc>] qla2x00_sysfs_read_optrom+0x9c/0xc0 [qla2xxx]
[ 1479.475571]  [<ffffffff8127e76b>] read+0xdb/0x1f0
[ 1479.476206]  [<ffffffff811fdf9e>] vfs_read+0x9e/0x170
[ 1479.476839]  [<ffffffff811feb6f>] SyS_read+0x7f/0xe0
[ 1479.477466]  [<ffffffff816964c9>] system_call_fastpath+0x16/0x1b

Below patch modifies qla2x00_sysfs_read_optrom,
qla2x00_sysfs_write_optrom functions to get the mutex_lock before
checking ha->optrom_state to avoid similar crashes.

The patch was applied and tested and same crashes were no longer
observed again.
Tested-by: default avatarMilan P. Gandhi <mgandhi@redhat.com>
Signed-off-by: default avatarMilan P. Gandhi <mgandhi@redhat.com>
Reviewed-by: default avatarLaurence Oberman <loberman@redhat.com>
Acked-by: default avatarHimanshu Madhani <himanshu.madhani@cavium.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: default avatarSasha Levin <alexander.levin@verizon.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 99eb27d6
...@@ -243,12 +243,15 @@ qla2x00_sysfs_read_optrom(struct file *filp, struct kobject *kobj, ...@@ -243,12 +243,15 @@ qla2x00_sysfs_read_optrom(struct file *filp, struct kobject *kobj,
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
ssize_t rval = 0; ssize_t rval = 0;
mutex_lock(&ha->optrom_mutex);
if (ha->optrom_state != QLA_SREADING) if (ha->optrom_state != QLA_SREADING)
return 0; goto out;
mutex_lock(&ha->optrom_mutex);
rval = memory_read_from_buffer(buf, count, &off, ha->optrom_buffer, rval = memory_read_from_buffer(buf, count, &off, ha->optrom_buffer,
ha->optrom_region_size); ha->optrom_region_size);
out:
mutex_unlock(&ha->optrom_mutex); mutex_unlock(&ha->optrom_mutex);
return rval; return rval;
...@@ -263,14 +266,19 @@ qla2x00_sysfs_write_optrom(struct file *filp, struct kobject *kobj, ...@@ -263,14 +266,19 @@ qla2x00_sysfs_write_optrom(struct file *filp, struct kobject *kobj,
struct device, kobj))); struct device, kobj)));
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
if (ha->optrom_state != QLA_SWRITING) mutex_lock(&ha->optrom_mutex);
if (ha->optrom_state != QLA_SWRITING) {
mutex_unlock(&ha->optrom_mutex);
return -EINVAL; return -EINVAL;
if (off > ha->optrom_region_size) }
if (off > ha->optrom_region_size) {
mutex_unlock(&ha->optrom_mutex);
return -ERANGE; return -ERANGE;
}
if (off + count > ha->optrom_region_size) if (off + count > ha->optrom_region_size)
count = ha->optrom_region_size - off; count = ha->optrom_region_size - off;
mutex_lock(&ha->optrom_mutex);
memcpy(&ha->optrom_buffer[off], buf, count); memcpy(&ha->optrom_buffer[off], buf, count);
mutex_unlock(&ha->optrom_mutex); mutex_unlock(&ha->optrom_mutex);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment