Smatch reports a warning as follows:
drivers/scsi/snic/snic_disc.c:307 snic_tgt_create() warn:
'&tgt->list' not removed from list
If device_add() fails in snic_tgt_create(), tgt will be freed, but
tgt->list will not be removed from snic->disc.tgt_list, then list traversal
may cause UAF.
Remove from snic->disc.tgt_list before free().
Fixes: c8806b6c9e ("snic: driver for Cisco SCSI HBA")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
Link: https://lore.kernel.org/r/20221117035100.2944812-1-cuigaosheng1@huawei.com
Acked-by: Narsimhulu Musini <nmusini@cisco.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Initialization of vha->unknown_atio_list and vha->unknown_atio_work only
happens for base_vha in qlt_probe_one_stage1(). But there is no
initialization for NPIV hosts that are created in qla24xx_vport_create().
This causes a crash when trying to access these NPIV host fields.
Fix this by adding initialization to qla_vport_create().
Signed-off-by: Gleb Chesnokov <gleb.chesnokov@scst.dev>
Link: https://lore.kernel.org/r/376c89a2-a9ac-bcf9-bf0f-dfe89a02fd4b@scst.dev
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Commit 9b3e0f4d41 ("scsi: qla2xxx: Move work element processing out of
DPC thread") introduced the initialization of vha->iocb_work in
qla2x00_create_host() function.
This initialization is also called from qla2x00_probe_one() function, just
after qla2x00_create_host().
Hence remove this duplicate call since it has already been called before.
Signed-off-by: Gleb Chesnokov <gleb.chesnokov@scst.dev>
Link: https://lore.kernel.org/r/822b3823-f344-67d6-30f1-16e31cf68eed@scst.dev
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
fcoe_init() calls fcoe_transport_attach(&fcoe_sw_transport), but when
fcoe_if_init() fails, &fcoe_sw_transport is not detached and leaves freed
&fcoe_sw_transport on fcoe_transports list. This causes panic when
reinserting module.
BUG: unable to handle page fault for address: fffffbfff82e2213
RIP: 0010:fcoe_transport_attach+0xe1/0x230 [libfcoe]
Call Trace:
<TASK>
do_one_initcall+0xd0/0x4e0
load_module+0x5eee/0x7210
...
Fixes: 78a582463c ("[SCSI] fcoe: convert fcoe.ko to become an fcoe transport provider driver")
Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
Link: https://lore.kernel.org/r/20221115092442.133088-1-chenzhongjin@huawei.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
ZBC Zoned Block Commands specification mandates SYNCHRONIZE CACHE(16) for
host-managed zoned block devices, but does not mandate SYNCHRONIZE
CACHE(10). Call SYNCHRONIZE CACHE(16) in place of SYNCHRONIZE CACHE(10) to
ensure that the command is always supported. For this purpose, add
use_16_for_sync flag to struct scsi_device in same manner as use_16_for_rw
flag.
To be precise, ZBC does not mandate SYNCHRONIZE CACHE(16) for host-aware
zoned block devices. However, modern devices should support 16-byte
commands. Hence, call SYNCHRONIZE CACHE (16) on both types of ZBC devices,
host-aware and host-managed. Of note is that READ(16) and WRITE(16) have
same story and they are already called for both types of ZBC devices.
Another note is that this patch depends on the fix commit ea045fd344
("ata: libata-scsi: fix SYNCHRONIZE CACHE (16) command failure").
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Link: https://lore.kernel.org/r/20221115002905.1709006-1-shinichiro.kawasaki@wdc.com
Reviewed-by: Damien Le Moal <damien.lemoal@opendource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Afer commit 1fa5ae857b ("driver core: get rid of struct device's bus_id
string array"), the name of device is allocated dynamically, it needs be
freed when device_register() returns error.
As comment of device_register() says, one should use put_device() to give
up the reference in the error path. Fix this by calling put_device(), then
the name can be freed in kobject_cleanup(), and sdbg_host is freed in
sdebug_release_adapter().
When the device release is not set, it means the device is not initialized.
We can not call put_device() in this case. Use kfree() to free memory.
Fixes: 1fa5ae857b ("driver core: get rid of struct device's bus_id string array")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Link: https://lore.kernel.org/r/20221112131010.3757845-1-yangyingliang@huawei.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
If device_register() returns an error, the name allocated by dev_set_name()
needs to be freed. As the comment of device_register() says, one should use
put_device() to give up the reference in the error path. Fix this by
calling put_device(), then the name can be freed in kobject_cleanup().
The 'fcf' is freed in fcoe_fcf_device_release(), so the kfree() in the
error path can be removed.
The 'ctlr' is freed in fcoe_ctlr_device_release(), so don't use the error
label, just return NULL after calling put_device().
Fixes: 9a74e884ee ("[SCSI] libfcoe: Add fcoe_sysfs")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Link: https://lore.kernel.org/r/20221112094310.3633291-1-yangyingliang@huawei.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
As 'alloc_len' is user controlled data, if user tries to allocate memory
larger than(>=) MAX_ORDER, then kcalloc() will fail, it creates a stack
trace and messes up dmesg with a warning.
Add __GFP_NOWARN in order to avoid too large allocation warning. This is
detected by static analysis using smatch.
Fixes: 7db0e0c819 ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command")
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Link: https://lore.kernel.org/r/20221112070612.2121535-1-harshit.m.mogalapalli@oracle.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
hpsa_sas_port_add_phy() does:
...
sas_phy_add() -> may return error here
sas_port_add_phy()
...
Whereas hpsa_free_sas_phy() does:
...
sas_port_delete_phy()
sas_phy_delete()
...
If hpsa_sas_port_add_phy() returns an error, hpsa_free_sas_phy() can not be
called to free the memory because the port and the phy have not been added
yet.
Replace hpsa_free_sas_phy() with sas_phy_free() and kfree() to avoid kernel
crash in this case.
Fixes: d04e62b9d6 ("hpsa: add in sas transport class")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Link: https://lore.kernel.org/r/20221110151129.394389-1-yangyingliang@huawei.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
In mpt3sas_transport_port_add(), if sas_rphy_add() returns error,
sas_rphy_free() needs be called to free the resource allocated in
sas_end_device_alloc(). Otherwise a kernel crash will happen:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000108
CPU: 45 PID: 37020 Comm: bash Kdump: loaded Tainted: G W 6.1.0-rc1+ #189
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : device_del+0x54/0x3d0
lr : device_del+0x37c/0x3d0
Call trace:
device_del+0x54/0x3d0
attribute_container_class_device_del+0x28/0x38
transport_remove_classdev+0x6c/0x80
attribute_container_device_trigger+0x108/0x110
transport_remove_device+0x28/0x38
sas_rphy_remove+0x50/0x78 [scsi_transport_sas]
sas_port_delete+0x30/0x148 [scsi_transport_sas]
do_sas_phy_delete+0x78/0x80 [scsi_transport_sas]
device_for_each_child+0x68/0xb0
sas_remove_children+0x30/0x50 [scsi_transport_sas]
sas_rphy_remove+0x38/0x78 [scsi_transport_sas]
sas_port_delete+0x30/0x148 [scsi_transport_sas]
do_sas_phy_delete+0x78/0x80 [scsi_transport_sas]
device_for_each_child+0x68/0xb0
sas_remove_children+0x30/0x50 [scsi_transport_sas]
sas_remove_host+0x20/0x38 [scsi_transport_sas]
scsih_remove+0xd8/0x420 [mpt3sas]
Because transport_add_device() is not called when sas_rphy_add() fails, the
device is not added. When sas_rphy_remove() is subsequently called to
remove the device in the remove() path, a NULL pointer dereference happens.
Fixes: f92363d123 ("[SCSI] mpt3sas: add new driver supporting 12GB SAS")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Link: https://lore.kernel.org/r/20221109032403.1636422-1-yangyingliang@huawei.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The hpda_alloc_ctlr_info() allocates h and its field reply_map. However, in
hpsa_init_one(), if alloc_percpu() failed, the hpsa_init_one() jumps to
clean1 directly, which frees h and leaks the h->reply_map.
Fix by calling hpda_free_ctlr_info() to release h->replay_map and h instead
free h directly.
Fixes: 8b834bff1b ("scsi: hpsa: fix selection of reply queue")
Signed-off-by: Yuan Can <yuancan@huawei.com>
Link: https://lore.kernel.org/r/20221122015751.87284-1-yuancan@huawei.com
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
If a SCSI command times out and is going to be aborted, we should increase
the iodone_cnt of the related scsi_device. Otherwise the iodone_cnt would
be smaller than iorequest_cnt.
Increasing iodone_cnt in scsi_timeout() would not cause a double accounting
issue. Brief analysis follows:
- We add the iodone_cnt when BLK_EH_DONE is returned in
scsi_timeout(). The related command's timeout event would not happen.
- If the abort succeeds and the command is not retried, the command would
be completed with scsi_finish_command() which would not increase
iodone_cnt.
- If the abort succeeds and the command is retried, it would be requeue. A
scsi_dispatch_cmd() would be called and iorequest_cnt would be increased
again.
- If the abort fails, the error handler successfully recovers the device,
and the command is not retried, the command would be completed with
scsi_finish_command() which would not increase iodone_cnt.
- If the abort fails, the error handler successfully recovers the device,
and the command is retried, the iorequest_cnt would be increased again.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Link: https://lore.kernel.org/r/20221123122137.150776-2-haowenchao@huawei.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
When a FLOGI completes with a sequence timeout error, a freed kref ptr
dereference crash can occur due to a timing race involving ndlp referencing
in lpfc_dev_loss_tmo_callbk.
Fix by ensuring the driver accounts for an outstanding FLOGI when dev_loss
is active. Also, don't remove the HBA_FLOGI_OUTSTANDING flag when the
FLOGI is retried to allow the driver to handle the reference counts
correctly in lpfc_dev_loss_tmo_handler.
Reported-by: Dietmar Hahn <dietmar.hahn@fujitsu.com>
Tested-by: Dietmar Hahn <dietmar.hahn@fujitsu.com>
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20221116011921.105995-5-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The dynamic mi_ver value holds the currently configured MI setting. mi_ver
was being displayed as part of the cmf_info sysfs attribute, when the
output string meant to display MI capabilities instead.
Add a mi_cap member in the lpfc_pc_sli4_params structure that will store MI
capabilities during initialization so that cmf_info prints out capabilities
instead of current configuration.
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20221116011921.105995-4-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The lpfc_cmf_timer adjusts phba->cmf_link_byte_count periodically and can
artifically inflate bandwidth percent.
During bandwidth calculation, correct for this by setting a cap of logging
a maximum of 100%.
Bandwidth calculation is only used for display under LOG_CGN_MGMT so there
is no expectation of impacts on performance.
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20221116011921.105995-3-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Adapter configurations with limited EQ resources may fail to initialize.
Firmware resources are queried in lpfc_sli4_read_config(). The driver
parameters cfg_irq_chann and cfg_hdw_queue are adjusted from defaults if
constrained by firmware resources.
The minimum resource check includes a special allocation for queues such as
ELS, MBOX, NVME LS. However the additional reservation was also incorrectly
applied to EQ resources.
Reordered WQ|CQ|EQ resource checks to apply the special allocation
adjustment to WQ and CQ resources only.
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20221116011921.105995-2-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
As 'lbdof_blen' is coming from user, if the size in kzalloc() is >=
MAX_ORDER then we hit a warning.
Call trace:
sg_ioctl
sg_ioctl_common
scsi_ioctl
sg_scsi_ioctl
blk_execute_rq
blk_mq_sched_insert_request
blk_mq_run_hw_queue
__blk_mq_delay_run_hw_queue
__blk_mq_run_hw_queue
blk_mq_sched_dispatch_requests
__blk_mq_sched_dispatch_requests
blk_mq_dispatch_rq_list
scsi_queue_rq
scsi_dispatch_cmd
scsi_debug_queuecommand
schedule_resp
resp_write_scat
If you try to allocate a memory larger than(>=) MAX_ORDER, then kmalloc()
will definitely fail. It creates a stack trace and messes up dmesg. The
user controls the size here so if they specify a too large size it will
fail.
Add __GFP_NOWARN in order to avoid too large allocation warning. This is
detected by static analysis using smatch.
Fixes: 481b5e5c79 ("scsi: scsi_debug: add resp_write_scat function")
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Link: https://lore.kernel.org/r/20221111100526.1790533-1-harshit.m.mogalapalli@oracle.com
Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Fix the following two compiler warnings:
drivers/scsi/qla2xxx/qla_init.c: In function ‘qla24xx_async_abort_cmd’:
drivers/scsi/qla2xxx/qla_init.c:171:17: warning: variable ‘bail’ set but not used [-Wunused-but-set-variable]
171 | uint8_t bail;
| ^~~~
drivers/scsi/qla2xxx/qla_init.c: In function ‘qla2x00_async_tm_cmd’:
drivers/scsi/qla2xxx/qla_init.c:2023:17: warning: variable ‘bail’ set but not used [-Wunused-but-set-variable]
2023 | uint8_t bail;
| ^~~~
Cc: Arun Easi <arun.easi@qlogic.com>
Cc: Giridhar Malavali <giridhar.malavali@qlogic.com>
Fixes: feafb7b171 ("[SCSI] qla2xxx: Fix vport delete issues")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/20221031224818.2607882-1-bvanassche@acm.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
indirect call targets are validated against the expected function pointer
prototype to make sure the call target is valid to help mitigate ROP
attacks. If they are not identical, there is a failure at run time, which
manifests as either a kernel panic or thread getting killed. A proposed
warning in clang aims to catch these at compile time, which reveals:
drivers/scsi/elx/libefc/efc_node.c:811:22: error: incompatible function pointer types assigning to 'void (*)(struct efc_sm_ctx *, u32, void *)' (aka 'void (*)(struct efc_sm_ctx *, unsigned int, void *)') from 'void (*)(struct efc_sm_ctx *, enum efc_sm_event, void *)' [-Werror,-Wincompatible-function-pointer-types-strict]
ctx->current_state = state;
^ ~~~~~
drivers/scsi/elx/libefc/efc_node.c:878:21: error: incompatible function pointer types assigning to 'void (*)(struct efc_sm_ctx *, u32, void *)' (aka 'void (*)(struct efc_sm_ctx *, unsigned int, void *)') from 'void (*)(struct efc_sm_ctx *, enum efc_sm_event, void *)' [-Werror,-Wincompatible-function-pointer-types-strict]
node->nodedb_state = state;
^ ~~~~~
drivers/scsi/elx/libefc/efc_node.c:905:6: error: incompatible function pointer types assigning to 'void (*)(struct efc_sm_ctx *, enum efc_sm_event, void *)' from 'void (*)(struct efc_sm_ctx *, u32, void *)' (aka 'void (*)(struct efc_sm_ctx *, unsigned int, void *)') [-Werror,-Wincompatible-function-pointer-types-strict]
pf = node->nodedb_state;
^ ~~~~~~~~~~~~~~~~~~
drivers/scsi/elx/libefc/efc_device.c:455:22: error: incompatible function pointer types assigning to 'void (*)(struct efc_sm_ctx *, u32, void *)' (aka 'void (*)(struct efc_sm_ctx *, unsigned int, void *)') from 'void (struct efc_sm_ctx *, enum efc_sm_event, void *)' [-Werror,-Wincompatible-function-pointer-types-strict]
node->nodedb_state = __efc_d_init;
^ ~~~~~~~~~~~~
drivers/scsi/elx/libefc/efc_sm.c:41:22: error: incompatible function pointer types assigning to 'void (*)(struct efc_sm_ctx *, u32, void *)' (aka 'void (*)(struct efc_sm_ctx *, unsigned int, void *)') from 'void (*)(struct efc_sm_ctx *, enum efc_sm_event, void *)' [-Werror,-Wincompatible-function-pointer-types-strict]
ctx->current_state = state;
^ ~~~~~
The type of the second parameter in the prototypes of ->current_state() and
->nodedb_state() ('u32') does not match the implementations, which have a
second parameter type of 'enum efc_sm_event'. Update the prototypes to have
the correct second parameter type, clearing up all the warnings and CFI
failures.
Link: https://github.com/ClangBuiltLinux/linux/issues/1750
Reported-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Link: https://lore.kernel.org/r/20221102161906.2781508-1-nathan@kernel.org
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
