• Milan P. Gandhi's avatar
    scsi: qla2xxx: Get mutex lock before checking optrom_state · 1e43b2d0
    Milan P. Gandhi authored
    
    [ 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>
    1e43b2d0
qla_attr.c 62 KB