* [PATCH v3 0/2] crypto: qat: fix couple crashes duing error handling @ 2021-03-12 16:22 Tong Zhang 2021-03-12 16:22 ` [PATCH v3 1/2] crypto: qat - dont release uninitialized resources Tong Zhang 2021-03-12 16:22 ` [PATCH v3 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 0 siblings, 2 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-12 16:22 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Fiona Trahe, Tong Zhang, qat-linux, linux-crypto, linux-kernel There are a couple of issues in qat error handling. Those drivers tries to release resources that is not initialized. This patch series tries to fix crashes caused by incorrect error handling. v2: removed excessive dump in commit log as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> v3: collect tags as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> Tong Zhang (2): crypto: qat - dont release uninitialized resources crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- 4 files changed, 19 insertions(+), 10 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 1/2] crypto: qat - dont release uninitialized resources 2021-03-12 16:22 [PATCH v3 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang @ 2021-03-12 16:22 ` Tong Zhang 2021-03-18 15:53 ` Giovanni Cabiddu 2021-03-12 16:22 ` [PATCH v3 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 1 sibling, 1 reply; 16+ messages in thread From: Tong Zhang @ 2021-03-12 16:22 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Fiona Trahe, Tong Zhang, qat-linux, linux-crypto, linux-kernel adf_vf_isr_resource_alloc() is not unwinding correctly when error happens and it trys to release uninitialized resources. To fix this, only release initialized resources. [ 1.792845] Trying to free already-free IRQ 11 [ 1.793091] WARNING: CPU: 0 PID: 182 at kernel/irq/manage.c:1821 free_irq+0x202/0x380 [ 1.801340] Call Trace: [ 1.801477] adf_vf_isr_resource_free+0x32/0xb0 [intel_qat] [ 1.801785] adf_vf_isr_resource_alloc+0x14d/0x150 [intel_qat] [ 1.802105] adf_dev_init+0xba/0x140 [intel_qat] Signed-off-by: Tong Zhang <ztong0001@gmail.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c index 38d316a42ba6..888388acb6bd 100644 --- a/drivers/crypto/qat/qat_common/adf_vf_isr.c +++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c @@ -261,17 +261,26 @@ int adf_vf_isr_resource_alloc(struct adf_accel_dev *accel_dev) goto err_out; if (adf_setup_pf2vf_bh(accel_dev)) - goto err_out; + goto err_disable_msi; if (adf_setup_bh(accel_dev)) - goto err_out; + goto err_cleanup_pf2vf_bh; if (adf_request_msi_irq(accel_dev)) - goto err_out; + goto err_cleanup_bh; return 0; + +err_cleanup_bh: + adf_cleanup_bh(accel_dev); + +err_cleanup_pf2vf_bh: + adf_cleanup_pf2vf_bh(accel_dev); + +err_disable_msi: + adf_disable_msi(accel_dev); + err_out: - adf_vf_isr_resource_free(accel_dev); return -EFAULT; } EXPORT_SYMBOL_GPL(adf_vf_isr_resource_alloc); -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/2] crypto: qat - dont release uninitialized resources 2021-03-12 16:22 ` [PATCH v3 1/2] crypto: qat - dont release uninitialized resources Tong Zhang @ 2021-03-18 15:53 ` Giovanni Cabiddu 2021-03-18 16:06 ` Giovanni Cabiddu 0 siblings, 1 reply; 16+ messages in thread From: Giovanni Cabiddu @ 2021-03-18 15:53 UTC (permalink / raw) To: Tong Zhang Cc: Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Fiona Trahe, qat-linux, linux-crypto, linux-kernel On Fri, Mar 12, 2021 at 11:22:02AM -0500, Tong Zhang wrote: > adf_vf_isr_resource_alloc() is not unwinding correctly when error > happens and it trys to release uninitialized resources. > To fix this, only release initialized resources. > > [ 1.792845] Trying to free already-free IRQ 11 > [ 1.793091] WARNING: CPU: 0 PID: 182 at kernel/irq/manage.c:1821 free_irq+0x202/0x380 > [ 1.801340] Call Trace: > [ 1.801477] adf_vf_isr_resource_free+0x32/0xb0 [intel_qat] > [ 1.801785] adf_vf_isr_resource_alloc+0x14d/0x150 [intel_qat] > [ 1.802105] adf_dev_init+0xba/0x140 [intel_qat] > > Signed-off-by: Tong Zhang <ztong0001@gmail.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> I would add also a Fixes tag: Fixes: dd0f368398ea ("crypto: qat - Add qat dh895xcc VF driver") Minor nit, typo in commit message: dont --> don't/do not Apart from that Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> -- Giovanni > --- > drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c > index 38d316a42ba6..888388acb6bd 100644 > --- a/drivers/crypto/qat/qat_common/adf_vf_isr.c > +++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c > @@ -261,17 +261,26 @@ int adf_vf_isr_resource_alloc(struct adf_accel_dev *accel_dev) > goto err_out; > > if (adf_setup_pf2vf_bh(accel_dev)) > - goto err_out; > + goto err_disable_msi; > > if (adf_setup_bh(accel_dev)) > - goto err_out; > + goto err_cleanup_pf2vf_bh; > > if (adf_request_msi_irq(accel_dev)) > - goto err_out; > + goto err_cleanup_bh; > > return 0; > + > +err_cleanup_bh: > + adf_cleanup_bh(accel_dev); > + > +err_cleanup_pf2vf_bh: > + adf_cleanup_pf2vf_bh(accel_dev); > + > +err_disable_msi: > + adf_disable_msi(accel_dev); > + > err_out: > - adf_vf_isr_resource_free(accel_dev); > return -EFAULT; > } > EXPORT_SYMBOL_GPL(adf_vf_isr_resource_alloc); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/2] crypto: qat - dont release uninitialized resources 2021-03-18 15:53 ` Giovanni Cabiddu @ 2021-03-18 16:06 ` Giovanni Cabiddu 2021-03-18 16:21 ` [PATCH v4 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: Giovanni Cabiddu @ 2021-03-18 16:06 UTC (permalink / raw) To: Tong Zhang Cc: Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Fiona Trahe, qat-linux, linux-crypto, linux-kernel On Thu, Mar 18, 2021 at 03:53:03PM +0000, Giovanni Cabiddu wrote: > On Fri, Mar 12, 2021 at 11:22:02AM -0500, Tong Zhang wrote: > > adf_vf_isr_resource_alloc() is not unwinding correctly when error > > happens and it trys to release uninitialized resources. ^^^^ Typo: perhaps 'tries'? Regards, -- Giovanni ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 0/2] crypto: qat: fix couple crashes duing error handling 2021-03-18 16:06 ` Giovanni Cabiddu @ 2021-03-18 16:21 ` Tong Zhang 2021-03-18 16:21 ` [PATCH v4 1/2] crypto: qat - don't release uninitialized resources Tong Zhang ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-18 16:21 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Tong Zhang, Fiona Trahe, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel, Wojciech Ziemba There are a couple of issues in qat error handling. Those drivers tries to release resources that is not initialized. This patch series tries to fix crashes caused by incorrect error handling. v2: removed excessive dump in commit log as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> v3: collect tags as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> v4: fix commit log typos Tong Zhang (2): crypto: qat - dont release uninitialized resources crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- 4 files changed, 19 insertions(+), 10 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 1/2] crypto: qat - don't release uninitialized resources 2021-03-18 16:06 ` Giovanni Cabiddu 2021-03-18 16:21 ` [PATCH v4 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang @ 2021-03-18 16:21 ` Tong Zhang 2021-03-18 16:21 ` [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-18 16:22 ` [PATCH v3 1/2] crypto: qat - dont release uninitialized resources Tong Zhang 3 siblings, 0 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-18 16:21 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Tong Zhang, Fiona Trahe, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel, Wojciech Ziemba adf_vf_isr_resource_alloc() is not unwinding correctly when error happens and it tris to release uninitialized resources. To fix this, only release initialized resources. [ 1.792845] Trying to free already-free IRQ 11 [ 1.793091] WARNING: CPU: 0 PID: 182 at kernel/irq/manage.c:1821 free_irq+0x202/0x380 [ 1.801340] Call Trace: [ 1.801477] adf_vf_isr_resource_free+0x32/0xb0 [intel_qat] [ 1.801785] adf_vf_isr_resource_alloc+0x14d/0x150 [intel_qat] [ 1.802105] adf_dev_init+0xba/0x140 [intel_qat] Signed-off-by: Tong Zhang <ztong0001@gmail.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Fixes: dd0f368398ea ("crypto: qat - Add qat dh895xcc VF driver") Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> --- drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c index 38d316a42ba6..888388acb6bd 100644 --- a/drivers/crypto/qat/qat_common/adf_vf_isr.c +++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c @@ -261,17 +261,26 @@ int adf_vf_isr_resource_alloc(struct adf_accel_dev *accel_dev) goto err_out; if (adf_setup_pf2vf_bh(accel_dev)) - goto err_out; + goto err_disable_msi; if (adf_setup_bh(accel_dev)) - goto err_out; + goto err_cleanup_pf2vf_bh; if (adf_request_msi_irq(accel_dev)) - goto err_out; + goto err_cleanup_bh; return 0; + +err_cleanup_bh: + adf_cleanup_bh(accel_dev); + +err_cleanup_pf2vf_bh: + adf_cleanup_pf2vf_bh(accel_dev); + +err_disable_msi: + adf_disable_msi(accel_dev); + err_out: - adf_vf_isr_resource_free(accel_dev); return -EFAULT; } EXPORT_SYMBOL_GPL(adf_vf_isr_resource_alloc); -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-18 16:06 ` Giovanni Cabiddu 2021-03-18 16:21 ` [PATCH v4 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang 2021-03-18 16:21 ` [PATCH v4 1/2] crypto: qat - don't release uninitialized resources Tong Zhang @ 2021-03-18 16:21 ` Tong Zhang 2021-03-18 17:17 ` Giovanni Cabiddu 2021-03-18 16:22 ` [PATCH v3 1/2] crypto: qat - dont release uninitialized resources Tong Zhang 3 siblings, 1 reply; 16+ messages in thread From: Tong Zhang @ 2021-03-18 16:21 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Tong Zhang, Fiona Trahe, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel, Wojciech Ziemba ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the vf2pf_lock is initialized in adf_dev_init(), which can fail and when it fail, the vf2pf_lock is either not initialized or destroyed, a subsequent use of vf2pf_lock will cause issue. To fix this issue, only set this flag if adf_dev_init() returns 0. [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 [ 7.180345] Call Trace: [ 7.182576] mutex_lock+0xc9/0xd0 [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] Signed-off-by: Tong Zhang <ztong0001@gmail.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Fixes: 25c6ffb249f6 ("crypto: qat - check if PF is running") Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> --- drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c index 1d1532e8fb6d..067ca5e17d38 100644 --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c index 04742a6d91ca..51ea88c0b17d 100644 --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c index c972554a755e..29999da716cc 100644 --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-18 16:21 ` [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang @ 2021-03-18 17:17 ` Giovanni Cabiddu 2021-03-19 3:39 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Tong Zhang 2021-03-19 3:41 ` [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 0 siblings, 2 replies; 16+ messages in thread From: Giovanni Cabiddu @ 2021-03-18 17:17 UTC (permalink / raw) To: Tong Zhang Cc: Herbert Xu, David S. Miller, Andy Shevchenko, Fiona Trahe, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel, Wojciech Ziemba Just a minor comment on the commit message: crypto: qat: ADF_STATUS_PF_RUNNING ... ^ Patches to the qat driver have the following headline: crypto: qat - not crypto: qat : Regards, -- Giovanni On Thu, Mar 18, 2021 at 12:21:05PM -0400, Tong Zhang wrote: > ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() > before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the > vf2pf_lock is initialized in adf_dev_init(), which can fail and when it > fail, the vf2pf_lock is either not initialized or destroyed, a subsequent > use of vf2pf_lock will cause issue. > To fix this issue, only set this flag if adf_dev_init() returns 0. > > [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 > [ 7.180345] Call Trace: > [ 7.182576] mutex_lock+0xc9/0xd0 > [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] > [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] > [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] > [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] > > Signed-off-by: Tong Zhang <ztong0001@gmail.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Fixes: 25c6ffb249f6 ("crypto: qat - check if PF is running") > Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > --- > drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > index 1d1532e8fb6d..067ca5e17d38 100644 > --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > index 04742a6d91ca..51ea88c0b17d 100644 > --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > index c972554a755e..29999da716cc 100644 > --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling 2021-03-18 17:17 ` Giovanni Cabiddu @ 2021-03-19 3:39 ` Tong Zhang 2021-03-19 3:39 ` [PATCH v5 1/2] crypto: qat - don't release uninitialized resources Tong Zhang ` (2 more replies) 2021-03-19 3:41 ` [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 1 sibling, 3 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-19 3:39 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Tong Zhang, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel There are a couple of issues in qat error handling. Those drivers tries to release resources that is not initialized. This patch series tries to fix crashes caused by incorrect error handling. v2: removed excessive dump in commit log as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> v3: collect tags as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> v4: fix commit log typos v5: fix headline Tong Zhang (2): crypto: qat - don't release uninitialized resources crypto: qat - ADF_STATUS_PF_RUNNING should be set after adf_dev_init drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- 4 files changed, 19 insertions(+), 10 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 1/2] crypto: qat - don't release uninitialized resources 2021-03-19 3:39 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Tong Zhang @ 2021-03-19 3:39 ` Tong Zhang 2021-03-19 3:40 ` [PATCH v5 2/2] crypto: qat - ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-26 9:31 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Herbert Xu 2 siblings, 0 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-19 3:39 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Tong Zhang, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel adf_vf_isr_resource_alloc() is not unwinding correctly when error happens and it want to release uninitialized resources. To fix this, only release initialized resources. [ 1.792845] Trying to free already-free IRQ 11 [ 1.793091] WARNING: CPU: 0 PID: 182 at kernel/irq/manage.c:1821 free_irq+0x202/0x380 [ 1.801340] Call Trace: [ 1.801477] adf_vf_isr_resource_free+0x32/0xb0 [intel_qat] [ 1.801785] adf_vf_isr_resource_alloc+0x14d/0x150 [intel_qat] [ 1.802105] adf_dev_init+0xba/0x140 [intel_qat] Signed-off-by: Tong Zhang <ztong0001@gmail.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Fixes: dd0f368398ea ("crypto: qat - Add qat dh895xcc VF driver") Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> --- drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c index 38d316a42ba6..888388acb6bd 100644 --- a/drivers/crypto/qat/qat_common/adf_vf_isr.c +++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c @@ -261,17 +261,26 @@ int adf_vf_isr_resource_alloc(struct adf_accel_dev *accel_dev) goto err_out; if (adf_setup_pf2vf_bh(accel_dev)) - goto err_out; + goto err_disable_msi; if (adf_setup_bh(accel_dev)) - goto err_out; + goto err_cleanup_pf2vf_bh; if (adf_request_msi_irq(accel_dev)) - goto err_out; + goto err_cleanup_bh; return 0; + +err_cleanup_bh: + adf_cleanup_bh(accel_dev); + +err_cleanup_pf2vf_bh: + adf_cleanup_pf2vf_bh(accel_dev); + +err_disable_msi: + adf_disable_msi(accel_dev); + err_out: - adf_vf_isr_resource_free(accel_dev); return -EFAULT; } EXPORT_SYMBOL_GPL(adf_vf_isr_resource_alloc); -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 2/2] crypto: qat - ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-19 3:39 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Tong Zhang 2021-03-19 3:39 ` [PATCH v5 1/2] crypto: qat - don't release uninitialized resources Tong Zhang @ 2021-03-19 3:40 ` Tong Zhang 2021-03-26 9:31 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Herbert Xu 2 siblings, 0 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-19 3:40 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Tong Zhang, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel, Fiona Trahe ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the vf2pf_lock is initialized in adf_dev_init(), which can fail and when it fail, the vf2pf_lock is either not initialized or destroyed, a subsequent use of vf2pf_lock will cause issue. To fix this issue, only set this flag if adf_dev_init() returns 0. [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 [ 7.180345] Call Trace: [ 7.182576] mutex_lock+0xc9/0xd0 [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] Signed-off-by: Tong Zhang <ztong0001@gmail.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Fixes: 25c6ffb249f6 ("crypto: qat - check if PF is running") Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> --- drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c index 1d1532e8fb6d..067ca5e17d38 100644 --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c index 04742a6d91ca..51ea88c0b17d 100644 --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c index c972554a755e..29999da716cc 100644 --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling 2021-03-19 3:39 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Tong Zhang 2021-03-19 3:39 ` [PATCH v5 1/2] crypto: qat - don't release uninitialized resources Tong Zhang 2021-03-19 3:40 ` [PATCH v5 2/2] crypto: qat - ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang @ 2021-03-26 9:31 ` Herbert Xu 2 siblings, 0 replies; 16+ messages in thread From: Herbert Xu @ 2021-03-26 9:31 UTC (permalink / raw) To: Tong Zhang Cc: Giovanni Cabiddu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Tadeusz Struk, qat-linux, linux-crypto, linux-kernel On Thu, Mar 18, 2021 at 11:39:58PM -0400, Tong Zhang wrote: > There are a couple of issues in qat error handling. Those drivers tries to > release resources that is not initialized. This patch series tries to fix > crashes caused by incorrect error handling. > > v2: removed excessive dump in commit log as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> > v3: collect tags as suggested by Andy Shevchenko <andy.shevchenko@gmail.com> > v4: fix commit log typos > v5: fix headline > > Tong Zhang (2): > crypto: qat - don't release uninitialized resources > crypto: qat - ADF_STATUS_PF_RUNNING should be set after adf_dev_init > > drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- > drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- > 4 files changed, 19 insertions(+), 10 deletions(-) All applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-18 17:17 ` Giovanni Cabiddu 2021-03-19 3:39 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Tong Zhang @ 2021-03-19 3:41 ` Tong Zhang 1 sibling, 0 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-19 3:41 UTC (permalink / raw) To: Giovanni Cabiddu Cc: Herbert Xu, David S. Miller, Andy Shevchenko, Fiona Trahe, Tadeusz Struk, qat-linux, linux-crypto, open list, Wojciech Ziemba Thanks, I fixed the headline and sent it as v5. - Tong On Thu, Mar 18, 2021 at 1:18 PM Giovanni Cabiddu <giovanni.cabiddu@intel.com> wrote: > > Just a minor comment on the commit message: > crypto: qat: ADF_STATUS_PF_RUNNING ... > ^ > Patches to the qat driver have the following headline: > crypto: qat - > not > crypto: qat : > > Regards, > > -- > Giovanni > > On Thu, Mar 18, 2021 at 12:21:05PM -0400, Tong Zhang wrote: > > ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() > > before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the > > vf2pf_lock is initialized in adf_dev_init(), which can fail and when it > > fail, the vf2pf_lock is either not initialized or destroyed, a subsequent > > use of vf2pf_lock will cause issue. > > To fix this issue, only set this flag if adf_dev_init() returns 0. > > > > [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 > > [ 7.180345] Call Trace: > > [ 7.182576] mutex_lock+0xc9/0xd0 > > [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] > > [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] > > [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] > > [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] > > > > Signed-off-by: Tong Zhang <ztong0001@gmail.com> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > Fixes: 25c6ffb249f6 ("crypto: qat - check if PF is running") > > Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > > --- > > drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- > > drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- > > drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- > > 3 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > > index 1d1532e8fb6d..067ca5e17d38 100644 > > --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > > +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (ret) > > goto out_err_free_reg; > > > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > - > > ret = adf_dev_init(accel_dev); > > if (ret) > > goto out_err_dev_shutdown; > > > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > + > > ret = adf_dev_start(accel_dev); > > if (ret) > > goto out_err_dev_stop; > > diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > > index 04742a6d91ca..51ea88c0b17d 100644 > > --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c > > +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (ret) > > goto out_err_free_reg; > > > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > - > > ret = adf_dev_init(accel_dev); > > if (ret) > > goto out_err_dev_shutdown; > > > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > + > > ret = adf_dev_start(accel_dev); > > if (ret) > > goto out_err_dev_stop; > > diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > > index c972554a755e..29999da716cc 100644 > > --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > > +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (ret) > > goto out_err_free_reg; > > > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > - > > ret = adf_dev_init(accel_dev); > > if (ret) > > goto out_err_dev_shutdown; > > > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > + > > ret = adf_dev_start(accel_dev); > > if (ret) > > goto out_err_dev_stop; > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/2] crypto: qat - dont release uninitialized resources 2021-03-18 16:06 ` Giovanni Cabiddu ` (2 preceding siblings ...) 2021-03-18 16:21 ` [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang @ 2021-03-18 16:22 ` Tong Zhang 3 siblings, 0 replies; 16+ messages in thread From: Tong Zhang @ 2021-03-18 16:22 UTC (permalink / raw) To: Giovanni Cabiddu Cc: Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Fiona Trahe, qat-linux, linux-crypto, open list Thanks! I have fixed typos and resent them as v4. - Tong ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-12 16:22 [PATCH v3 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang 2021-03-12 16:22 ` [PATCH v3 1/2] crypto: qat - dont release uninitialized resources Tong Zhang @ 2021-03-12 16:22 ` Tong Zhang 2021-03-18 16:03 ` Giovanni Cabiddu 1 sibling, 1 reply; 16+ messages in thread From: Tong Zhang @ 2021-03-12 16:22 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Fiona Trahe, Tong Zhang, qat-linux, linux-crypto, linux-kernel ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the vf2pf_lock is initialized in adf_dev_init(), which can fail and when it fail, the vf2pf_lock is either not initialized or destroyed, a subsequent use of vf2pf_lock will cause issue. To fix this issue, only set this flag if adf_dev_init() returns 0. [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 [ 7.180345] Call Trace: [ 7.182576] mutex_lock+0xc9/0xd0 [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] Signed-off-by: Tong Zhang <ztong0001@gmail.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c index 1d1532e8fb6d..067ca5e17d38 100644 --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c index 04742a6d91ca..51ea88c0b17d 100644 --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c index c972554a755e..29999da716cc 100644 --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-12 16:22 ` [PATCH v3 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang @ 2021-03-18 16:03 ` Giovanni Cabiddu 0 siblings, 0 replies; 16+ messages in thread From: Giovanni Cabiddu @ 2021-03-18 16:03 UTC (permalink / raw) To: Tong Zhang Cc: Herbert Xu, David S. Miller, Andy Shevchenko, Wojciech Ziemba, Fiona Trahe, qat-linux, linux-crypto, linux-kernel On Fri, Mar 12, 2021 at 11:22:03AM -0500, Tong Zhang wrote: > ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() > before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the > vf2pf_lock is initialized in adf_dev_init(), which can fail and when it > fail, the vf2pf_lock is either not initialized or destroyed, a subsequent > use of vf2pf_lock will cause issue. > To fix this issue, only set this flag if adf_dev_init() returns 0. > > [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 > [ 7.180345] Call Trace: > [ 7.182576] mutex_lock+0xc9/0xd0 > [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] > [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] > [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] > [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] > > Signed-off-by: Tong Zhang <ztong0001@gmail.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Fixes: 25c6ffb249f6 ("crypto: qat - check if PF is running") Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > --- > drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > index 1d1532e8fb6d..067ca5e17d38 100644 > --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > index 04742a6d91ca..51ea88c0b17d 100644 > --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > index c972554a755e..29999da716cc 100644 > --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-03-26 9:32 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-12 16:22 [PATCH v3 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang 2021-03-12 16:22 ` [PATCH v3 1/2] crypto: qat - dont release uninitialized resources Tong Zhang 2021-03-18 15:53 ` Giovanni Cabiddu 2021-03-18 16:06 ` Giovanni Cabiddu 2021-03-18 16:21 ` [PATCH v4 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang 2021-03-18 16:21 ` [PATCH v4 1/2] crypto: qat - don't release uninitialized resources Tong Zhang 2021-03-18 16:21 ` [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-18 17:17 ` Giovanni Cabiddu 2021-03-19 3:39 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Tong Zhang 2021-03-19 3:39 ` [PATCH v5 1/2] crypto: qat - don't release uninitialized resources Tong Zhang 2021-03-19 3:40 ` [PATCH v5 2/2] crypto: qat - ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-26 9:31 ` [PATCH v5 0/2] crypto: qat - fix couple crashes duing error handling Herbert Xu 2021-03-19 3:41 ` [PATCH v4 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-18 16:22 ` [PATCH v3 1/2] crypto: qat - dont release uninitialized resources Tong Zhang 2021-03-12 16:22 ` [PATCH v3 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-18 16:03 ` Giovanni Cabiddu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.