In qla4xxx_probe_adapter, &ha->dpc_work is bound with
qla4xxx_do_dpc. qla4xxx_post_aen_work may be called
to start the work.
If we remove the module which will call qla4xxx_remove_adapter
to make cleanup, there may be a unfinished work. The possible
sequence is as follows:
Fix it by canceling the work before cleanup in qla4xxx_remove_adapter
CPU0 CPUc1
|qla4xxx_do_dpc
qla4xxx_remove_adapter|
scsi_remove_host |
kfree(ha->host) |
|
| iscsi_host_for_each_session
| //use ha->host
Fixes: afaf5a2d341d ("[SCSI] Initial Commit of qla4xxx")
Signed-off-by: Zheng Wang <[email protected]>
---
drivers/scsi/qla4xxx/ql4_os.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 005502125b27..d530cc853f43 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -9001,6 +9001,7 @@ static void qla4xxx_remove_adapter(struct pci_dev *pdev)
if (!pci_is_enabled(pdev))
return;
+ cancel_work_sync(&ha->dpc_work);
ha = pci_get_drvdata(pdev);
if (is_qla40XX(ha))
--
2.25.1
Sorry about the fix. This is a false positive which should be ignored.
The work will be posted on ha->dpc_thread. In qla4xxx_remove_adapter,
it will call destroy_workqueue(ha->dpc_thread);
Regards,
Zheng
Zheng Wang <[email protected]> 于2023年3月10日周五 02:38写道：
>
> In qla4xxx_probe_adapter, &ha->dpc_work is bound with
> qla4xxx_do_dpc. qla4xxx_post_aen_work may be called
> to start the work.
>
> If we remove the module which will call qla4xxx_remove_adapter
> to make cleanup, there may be a unfinished work. The possible
> sequence is as follows:
>
> Fix it by canceling the work before cleanup in qla4xxx_remove_adapter
>
> CPU0 CPUc1
>
> |qla4xxx_do_dpc
> qla4xxx_remove_adapter|
> scsi_remove_host |
> kfree(ha->host) |
> |
> | iscsi_host_for_each_session
> | //use ha->host
> Fixes: afaf5a2d341d ("[SCSI] Initial Commit of qla4xxx")
> Signed-off-by: Zheng Wang <[email protected]>
> ---
> drivers/scsi/qla4xxx/ql4_os.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 005502125b27..d530cc853f43 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -9001,6 +9001,7 @@ static void qla4xxx_remove_adapter(struct pci_dev *pdev)
> if (!pci_is_enabled(pdev))
> return;
>
> + cancel_work_sync(&ha->dpc_work);
> ha = pci_get_drvdata(pdev);
>
> if (is_qla40XX(ha))
> --
> 2.25.1
>
Hi Zheng,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v6.3-rc1 next-20230309]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zheng-Wang/scsi-qla4xxx-Fix-use-after-free-bug-in-da9150_charger_remove-due-to-race-condition/20230310-024016
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link: https://lore.kernel.org/r/20230309183812.299349-1-zyytlz.wz%40163.com
patch subject: [PATCH] scsi: qla4xxx: Fix use after free bug in da9150_charger_remove due to race condition
config: s390-randconfig-r044-20230308 (https://download.01.org/0day-ci/archive/20230310/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/f5410be5e51edb79365b3e0be8c80e4ff34e7e50
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Zheng-Wang/scsi-qla4xxx-Fix-use-after-free-bug-in-da9150_charger_remove-due-to-race-condition/20230310-024016
git checkout f5410be5e51edb79365b3e0be8c80e4ff34e7e50
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/scsi/qla4xxx/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/scsi/qla4xxx/ql4_os.c:10:
In file included from include/linux/inet.h:42:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from drivers/scsi/qla4xxx/ql4_os.c:10:
In file included from include/linux/inet.h:42:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from drivers/scsi/qla4xxx/ql4_os.c:10:
In file included from include/linux/inet.h:42:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/scsi/qla4xxx/ql4_os.c:9004:20: warning: variable 'ha' is uninitialized when used here [-Wuninitialized]
cancel_work_sync(&ha->dpc_work);
^~
drivers/scsi/qla4xxx/ql4_os.c:8995:26: note: initialize the variable 'ha' to silence this warning
struct scsi_qla_host *ha;
^
= NULL
13 warnings generated.
vim +/ha +9004 drivers/scsi/qla4xxx/ql4_os.c
8963
8964 static void qla4xxx_destroy_fw_ddb_session(struct scsi_qla_host *ha)
8965 {
8966 struct ddb_entry *ddb_entry;
8967 int idx;
8968
8969 for (idx = 0; idx < MAX_DDB_ENTRIES; idx++) {
8970
8971 ddb_entry = qla4xxx_lookup_ddb_by_fw_index(ha, idx);
8972 if ((ddb_entry != NULL) &&
8973 (ddb_entry->ddb_type == FLASH_DDB)) {
8974
8975 qla4xxx_destroy_ddb(ha, ddb_entry);
8976 /*
8977 * we have decremented the reference count of the driver
8978 * when we setup the session to have the driver unload
8979 * to be seamless without actually destroying the
8980 * session
8981 **/
8982 try_module_get(qla4xxx_iscsi_transport.owner);
8983 iscsi_destroy_endpoint(ddb_entry->conn->ep);
8984 qla4xxx_free_ddb(ha, ddb_entry);
8985 iscsi_session_teardown(ddb_entry->sess);
8986 }
8987 }
8988 }
8989 /**
8990 * qla4xxx_remove_adapter - callback function to remove adapter.
8991 * @pdev: PCI device pointer
8992 **/
8993 static void qla4xxx_remove_adapter(struct pci_dev *pdev)
8994 {
8995 struct scsi_qla_host *ha;
8996
8997 /*
8998 * If the PCI device is disabled then it means probe_adapter had
8999 * failed and resources already cleaned up on probe_adapter exit.
9000 */
9001 if (!pci_is_enabled(pdev))
9002 return;
9003
> 9004 cancel_work_sync(&ha->dpc_work);
9005 ha = pci_get_drvdata(pdev);
9006
9007 if (is_qla40XX(ha))
9008 qla4xxx_prevent_other_port_reinit(ha);
9009
9010 /* destroy iface from sysfs */
9011 qla4xxx_destroy_ifaces(ha);
9012
9013 if ((!ql4xdisablesysfsboot) && ha->boot_kset)
9014 iscsi_boot_destroy_kset(ha->boot_kset);
9015
9016 qla4xxx_destroy_fw_ddb_session(ha);
9017 qla4_8xxx_free_sysfs_attr(ha);
9018
9019 qla4xxx_sysfs_ddb_remove(ha);
9020 scsi_remove_host(ha->host);
9021
9022 qla4xxx_free_adapter(ha);
9023
9024 scsi_host_put(ha->host);
9025
9026 pci_disable_pcie_error_reporting(pdev);
9027 pci_disable_device(pdev);
9028 }
9029
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests