1. 16 Apr, 2019 12 commits
    • Ming Lei's avatar
      scsi: core: don't hold device refcount in IO path · 18c4f0a4
      Ming Lei authored
      scsi_device's refcount is always grabbed in IO path.
      
      Turns out it isn't necessary, because blk_queue_cleanup() will drain any
      in-flight IOs, then cancel timeout/requeue work, and SCSI's requeue_work is
      canceled too in __scsi_remove_device().
      
      Also scsi_device won't go away until blk_cleanup_queue() is done.
      
      So don't hold the refcount in IO path, especially the refcount isn't
      required in IO path since blk_queue_enter() / blk_queue_exit() is
      introduced in the legacy block layer.
      
      Cc: Dongli Zhang <dongli.zhang@oracle.com>
      Cc: James Smart <james.smart@broadcom.com>
      Cc: Bart Van Assche <bart.vanassche@wdc.com>
      Cc: linux-scsi@vger.kernel.org,
      Cc: Martin K . Petersen <martin.petersen@oracle.com>,
      Cc: Christoph Hellwig <hch@lst.de>,
      Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>,
      Cc: jianchao wang <jianchao.w.wang@oracle.com>
      Reviewed-by: default avatarBart Van Assche <bvanassche@acm.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      18c4f0a4
    • Himanshu Madhani's avatar
      scsi: qla2xxx: Fix read offset in qla24xx_load_risc_flash() · 1710ac17
      Himanshu Madhani authored
      This patch fixes regression introduced by commit f8f97b0c ("scsi:
      qla2xxx: Cleanups for NVRAM/Flash read/write path") where flash read/write
      routine cleanup left out code which resulted into checksum failure leading
      to use-after-free stack during driver load.
      
      Following stack trace is seen in the log file
      
      qla2xxx [0000:00:00.0]-0005: : QLogic Fibre Channel HBA Driver: 10.01.00.16-k.
      qla2xxx [0000:00:0b.0]-001d: : Found an ISP2532 irq 11 iobase 0x0000000000f47f03.
      qla2xxx [0000:00:0b.0]-00cd:8: ISP Firmware failed checksum.
      qla2xxx [0000:00:0b.0]-00cf:8: Setup chip ****FAILED****.
      qla2xxx [0000:00:0b.0]-00d6:8: Failed to initialize adapter - Adapter flags 2.
      ==================================================================
      BUG: KASAN: use-after-free in __list_del_entry_valid+0x15/0xd0
      Read of size 8 at addr ffff8880ca05a490 by task modprobe/857
      
      CPU: 0 PID: 857 Comm: modprobe Not tainted 5.1.0-rc1-dbg+ #4
      Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
      Call Trace:
        dump_stack+0x86/0xca
        print_address_description+0x6c/0x234
        ? __list_del_entry_valid+0x15/0xd0
        kasan_report.cold.3+0x1b/0x34
        ? __list_del_entry_valid+0x15/0xd0
        ? __kmem_cache_shutdown.cold.95+0xf5/0x176
        ? __list_del_entry_valid+0x15/0xd0
        __asan_load8+0x54/0x90
        __list_del_entry_valid+0x15/0xd0
        dma_pool_destroy+0x4f/0x260
        ? dma_free_attrs+0xb4/0xd0
        qla2x00_mem_free+0x529/0xcc0 [qla2xxx]
        ? kobject_put+0xdb/0x230
        qla2x00_probe_one+0x2b5e/0x45f0 [qla2xxx]
        ? qla2xxx_pci_error_detected+0x210/0x210 [qla2xxx]
        ? match_held_lock+0x20/0x240
        ? find_held_lock+0xca/0xf0
        ? mark_held_locks+0x86/0xb0
        ? _raw_spin_unlock_irqrestore+0x52/0x60
        ? __pm_runtime_resume+0x5b/0xb0
        ? lockdep_hardirqs_on+0x185/0x260
        ? _raw_spin_unlock_irqrestore+0x52/0x60
        ? trace_hardirqs_on+0x24/0x130
        ? preempt_count_sub+0x13/0xc0
        ? _raw_spin_unlock_irqrestore+0x3d/0x60
        pci_device_probe+0x154/0x1e0
        really_probe+0x17d/0x540
        ? device_driver_attach+0x90/0x90
        driver_probe_device+0x113/0x170
        ? device_driver_attach+0x90/0x90
        device_driver_attach+0x88/0x90
        __driver_attach+0xb5/0x190
        bus_for_each_dev+0xf8/0x160
        ? subsys_dev_iter_exit+0x10/0x10
        ? kasan_check_read+0x11/0x20
        ? preempt_count_sub+0x13/0xc0
        ? _raw_spin_unlock+0x2c/0x50
        driver_attach+0x26/0x30
        bus_add_driver+0x238/0x2f0
        driver_register+0xd7/0x150
        __pci_register_driver+0xd5/0xe0
        ? 0xffffffffa06c8000
        qla2x00_module_init+0x208/0x254 [qla2xxx]
        do_one_initcall+0xc0/0x3c9
        ? trace_event_raw_event_initcall_finish+0x150/0x150
        ? __kasan_kmalloc.constprop.5+0xc7/0xd0
        ? kasan_unpoison_shadow+0x35/0x50
        ? kasan_poison_shadow+0x2f/0x40
        ? __asan_register_globals+0x5a/0x70
        do_init_module+0x103/0x330
        load_module+0x36df/0x3b70
        ? fsnotify+0x611/0x640
        ? module_frob_arch_sections+0x20/0x20
        ? kernel_read+0x74/0xa0
        ? kasan_check_write+0x14/0x20
        ? kernel_read_file+0x25e/0x320
        ? do_mmap+0x42c/0x6c0
        __do_sys_finit_module+0x133/0x1c0
        ? __do_sys_finit_module+0x133/0x1c0
        ? __do_sys_init_module+0x210/0x210
        ? fput_many+0x1b/0xc0
        ? fput+0xe/0x10
        ? do_syscall_64+0x14/0x210
        ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
        __x64_sys_finit_module+0x3e/0x50
        do_syscall_64+0x72/0x210
        entry_SYSCALL_64_after_hwframe+0x49/0xbe
      RIP: 0033:0x7f8bd5c03219
      Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 47 fc 0c 00 f7 d8 64 89 01 48
      RSP: 002b:00007fff9d11de98 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
      RAX: ffffffffffffffda RBX: 000055ef21596b50 RCX: 00007f8bd5c03219
      RDX: 0000000000000000 RSI: 000055ef21596570 RDI: 0000000000000004
      RBP: 000055ef21596570 R08: 0000000000000000 R09: 0000000000000000
      R10: 0000000000000004 R11: 0000000000000246 R12: 0000000000000000
      R13: 000055ef21596c80 R14: 0000000000040000 R15: 000055ef21596b50
      
      Allocated by task 857:
        save_stack+0x43/0xd0
        __kasan_kmalloc.constprop.5+0xc7/0xd0
        kasan_kmalloc+0x9/0x10
        kmem_cache_alloc_trace+0x144/0x300
        dma_pool_create+0xb5/0x3b0
        qla2x00_mem_alloc+0xb98/0x1ad0 [qla2xxx]
        qla2x00_probe_one+0xe28/0x45f0 [qla2xxx]
        pci_device_probe+0x154/0x1e0
        really_probe+0x17d/0x540
        driver_probe_device+0x113/0x170
        device_driver_attach+0x88/0x90
        __driver_attach+0xb5/0x190
        bus_for_each_dev+0xf8/0x160
        driver_attach+0x26/0x30
        bus_add_driver+0x238/0x2f0
        driver_register+0xd7/0x150
        __pci_register_driver+0xd5/0xe0
        qla2x00_module_init+0x208/0x254 [qla2xxx]
        do_one_initcall+0xc0/0x3c9
        do_init_module+0x103/0x330
        load_module+0x36df/0x3b70
        __do_sys_finit_module+0x133/0x1c0
        __x64_sys_finit_module+0x3e/0x50
        do_syscall_64+0x72/0x210
        entry_SYSCALL_64_after_hwframe+0x49/0xbe
      
      Freed by task 857:
        save_stack+0x43/0xd0
        __kasan_slab_free+0x139/0x190
        kasan_slab_free+0xe/0x10
        kfree+0xf0/0x2c0
        dma_pool_destroy+0x24c/0x260
        qla2x00_mem_free+0x529/0xcc0 [qla2xxx]
        qla2x00_free_device+0x167/0x1b0 [qla2xxx]
        qla2x00_probe_one+0x2b28/0x45f0 [qla2xxx]
        pci_device_probe+0x154/0x1e0
        really_probe+0x17d/0x540
        driver_probe_device+0x113/0x170
        device_driver_attach+0x88/0x90
        __driver_attach+0xb5/0x190
        bus_for_each_dev+0xf8/0x160
        driver_attach+0x26/0x30
        bus_add_driver+0x238/0x2f0
        driver_register+0xd7/0x150
        __pci_register_driver+0xd5/0xe0
        qla2x00_module_init+0x208/0x254 [qla2xxx]
        do_one_initcall+0xc0/0x3c9
        do_init_module+0x103/0x330
        load_module+0x36df/0x3b70
        __do_sys_finit_module+0x133/0x1c0
        __x64_sys_finit_module+0x3e/0x50
        do_syscall_64+0x72/0x210
        entry_SYSCALL_64_after_hwframe+0x49/0xbe
      
      The buggy address belongs to the object at ffff8880ca05a400
        which belongs to the cache kmalloc-192 of size 192
      The buggy address is located 144 bytes inside of
        192-byte region [ffff8880ca05a400, ffff8880ca05a4c0)
      The buggy address belongs to the page:
      page:ffffea0003281680 count:1 mapcount:0 mapping:ffff88811bf03380 index:0x0 compound_mapcount: 0
      flags: 0x4000000000010200(slab|head)
      raw: 4000000000010200 0000000000000000 0000000c00000001 ffff88811bf03380
      raw: 0000000000000000 0000000080200020 00000001ffffffff 0000000000000000
      page dumped because: kasan: bad access detected
      
      Memory state around the buggy address:
        ffff8880ca05a380: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
        ffff8880ca05a400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
      >ffff8880ca05a480: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
                                ^
        ffff8880ca05a500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        ffff8880ca05a580: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
      ==================================================================
      
      Fixes: f8f97b0c ("scsi: qla2xxx: Cleanups for NVRAM/Flash read/write path")
      Reported-by: default avatarBart Van Assche <bvanassche@acm.org>
      Tested-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      1710ac17
    • Bart Van Assche's avatar
      scsi: qla2xxx: Move qla2x00_set_fcport_state() from a .h into a .c file · a630bdc5
      Bart Van Assche authored
      The qla2x00_set_fcport_state() function is not in the hot path so move its
      definition from a .h into a .c file.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      a630bdc5
    • Bart Van Assche's avatar
      scsi: qla2xxx: Remove two superfluous casts · 81bcf1c5
      Bart Van Assche authored
      Casting a void pointer into another pointer before assigning the pointer to
      a variable is not useful. Hence remove such casts.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      81bcf1c5
    • Bart Van Assche's avatar
      scsi: qla2xxx: Remove qla_tgt_cmd.data_work and qla_tgt_cmd.data_work_free · bb63e47b
      Bart Van Assche authored
      The 'data_work' and 'data_work_free' member variables are set but never
      used. Hence remove both member variables. See also commit 6bcbb317
      ("qla2xxx: Fix incorrect tcm_qla2xxx_free_cmd use during TMR ABORT (v2)").
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      bb63e47b
    • Bart Van Assche's avatar
      scsi: qla2xxx: Move the <linux/io-64-nonatomic-lo-hi.h> include directive · 9dfb59a0
      Bart Van Assche authored
      The <linux/io-64-nonatomic-lo-hi.h> header file is included because of the
      readq() macro. Since that macro is only used in qla_nx.c, move that include
      statement into qla_nx.c.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      9dfb59a0
    • Bart Van Assche's avatar
      scsi: qla2xxx: Declare qla24xx_build_scsi_crc_2_iocbs() static · c20605ed
      Bart Van Assche authored
      Since qla24xx_build_scsi_crc_2_iocbs() is only used inside a single source
      file, declare this function static.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      c20605ed
    • Bart Van Assche's avatar
      scsi: qla2xxx: Move the port_state_str[] definition from a .h to a .c file · c4dc7cd3
      Bart Van Assche authored
      Reduce the size of the qla2xxx kernel module by moving an array definition
      from a .h into a .c file.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      c4dc7cd3
    • Bart Van Assche's avatar
      scsi: qla2xxx: Insert spaces where required · 58e2753c
      Bart Van Assche authored
      Improve source code readability by inserting spaces where these are
      required according to the coding standard. This patch only inserts
      whitespace and does not make any other changes.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      58e2753c
    • Bart Van Assche's avatar
      scsi: qla2xxx: Fix formatting of pointer types · 845bbb09
      Bart Van Assche authored
      Improve source code readability by following the Linux kernel coding style
      for pointer types. This patch only changes whitespace.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      845bbb09
    • Bart Van Assche's avatar
      scsi: qla2xxx: Leave a blank line after declarations · bd432bb5
      Bart Van Assche authored
      This patch improves readability of the qla2xxx source code.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      bd432bb5
    • Bart Van Assche's avatar
      scsi: qla2xxx: Use tabs to indent code · 2703eaaf
      Bart Van Assche authored
      Most but not all code in the qla2xxx driver uses tabs for indentation.
      Make the qla2xxx code easier to read by using tabs consistently for
      indentation. This patch improves conformance with the Linux kernel coding
      style.
      
      Cc: Himanshu Madhani <hmadhani@marvell.com>
      Cc: Giridhar Malavali <gmalavali@marvell.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Acked-by: default avatarHimanshu Madhani <hmadhani@marvell.com>
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      2703eaaf
  2. 15 Apr, 2019 6 commits
    • John Garry's avatar
      scsi: libsas: Print expander PHY indexes in decimal · 3c236f8c
      John Garry authored
      Currently we print expander PHY indexes in a mix of decimal and hex.
      
      It is more consistent and also more convenient to read decimal, so
      make this change.
      
      We use width of 2 for expander and 1 for root PHYs prints.
      
      Some lines which were needlessly spilling multiple lines are unified.
      Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      3c236f8c
    • John Garry's avatar
      scsi: libsas: Do discovery on empty PHY to update PHY info · d8649fc1
      John Garry authored
      When we discover the PHY is empty in sas_rediscover_dev(), the PHY
      information (like negotiated linkrate) is not updated.
      
      As such, for a user examining sysfs for that PHY, they would see
      incorrect values:
      
      root@(none)$ cd /sys/class/sas_phy/phy-0:0:20
      root@(none)$ more negotiated_linkrate
      3.0 Gbit
      root@(none)$ echo 0 > enable
      root@(none)$ more negotiated_linkrate
      3.0 Gbit
      
      So fix this, simply discover the PHY again, even though we know it's empty;
      in the above example, this gives us:
      
      root@(none)$ more negotiated_linkrate
      Phy disabled
      
      We must do this after unregistering the device associated with the PHY
      (in sas_unregister_devs_sas_addr()).
      Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      d8649fc1
    • John Garry's avatar
      scsi: libsas: Inject revalidate event for root port event · 085f104a
      John Garry authored
      According to the SAS spec, an expander device shall transmit BROADCAST
      (CHANGE) from at least one phy in each expander port other than the
      expander port that is the cause for transmitting BROADCAST (CHANGE).
      
      As such, for when the link is lost for a root PHY attached to an expander
      PHY, we get no broadcast event.
      
      This causes an issue for libsas, in that we will not revalidate the domain
      for these events.
      
      As a solution, for when a root PHY is formed or deformed from a root port,
      insert a broadcast event to trigger a domain revalidation.
      Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      085f104a
    • John Garry's avatar
      scsi: libsas: Improve vague log in SAS rediscovery · a5b38d31
      John Garry authored
      When an expander PHY which was part of a wideport disconnects, we would see
      a log like this from sas_rediscover():
      
      [   39.695554] sas: phy20 part of wide port with phy16
      
      Here, phy20 is the PHY that disconnected, and phy16 is the lowest indexed
      member PHY of the wideport.
      
      The log implies the phy20 is still part of the wideport with phy16, so is
      misleading or, at least, vague.
      
      Improve the logs in SAS rediscovery by removing this log and adding a log
      in sas_rediscover_dev() to tell what's really going on.
      
      While we're at it, also make the logs in sas_find_bcast_dev() more
      informative (and more consistent with the reset of the expander logs).
      Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      a5b38d31
    • John Garry's avatar
      scsi: libsas: Try to retain programmed min linkrate for SATA min pathway unmatch fixing · f7ddb43e
      John Garry authored
      Currently for fixing the linkrate matching during discovery such that the
      linkrate of a SATA PHY does not exceed min pathway to initiator, we set the
      SATA PHY programmed min linkrate to the same value as the programmed max
      linkrate.
      
      This is unnecessary, and we should be able to keep the same programmed min
      linkrate if it is already lower than this new max programmed linkrate.
      
      This patch makes that change.
      
      In effect, this will not make much difference since we generally will
      negotiate a linkrate at the programmed max linkrate, and the programmed min
      linkrate will have no impact.
      Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      f7ddb43e
    • John Garry's avatar
      scsi: libsas: Stop hardcoding SAS address length · 7b27c5fe
      John Garry authored
      Many times we use 8 for SAS address length, while we already have a macro
      for this - SAS_ADDR_SIZE.
      
      Replace instances of this with the macro. However, don't touch the SAS
      address array sizes sas.h, as these are defined according to the SAS spec.
      
      Some missing whitespaces are also added, and whitespace indentation
      in sas_hash_addr() is also fixed (see sas_hash_addr()).
      Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
      Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      7b27c5fe
  3. 13 Apr, 2019 22 commits