2021-05-27 19:15:59

by Chiappero, Marco

[permalink] [raw]
Subject: [PATCH 01/10] crypto: qat - use proper type for vf_mask

From: Giovanni Cabiddu <[email protected]>

Replace vf_mask type with unsigned long to avoid a stack-out-of-bound.

This is to fix the following warning reported by KASAN the first time
adf_msix_isr_ae() gets called.

[ 692.091987] BUG: KASAN: stack-out-of-bounds in find_first_bit+0x28/0x50
[ 692.092017] Read of size 8 at addr ffff88afdf789e60 by task swapper/32/0
[ 692.092076] Call Trace:
[ 692.092089] <IRQ>
[ 692.092101] dump_stack+0x9c/0xcf
[ 692.092132] print_address_description.constprop.0+0x18/0x130
[ 692.092164] ? find_first_bit+0x28/0x50
[ 692.092185] kasan_report.cold+0x7f/0x111
[ 692.092213] ? static_obj+0x10/0x80
[ 692.092234] ? find_first_bit+0x28/0x50
[ 692.092262] find_first_bit+0x28/0x50
[ 692.092288] adf_msix_isr_ae+0x16e/0x230 [intel_qat]

Fixes: ed8ccaef52fa ("crypto: qat - Add support for SRIOV")
Signed-off-by: Giovanni Cabiddu <[email protected]>
Reviewed-by: Marco Chiappero <[email protected]>
---
drivers/crypto/qat/qat_common/adf_isr.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/adf_isr.c b/drivers/crypto/qat/qat_common/adf_isr.c
index e3ad5587be49..22f8ef5bfbc5 100644
--- a/drivers/crypto/qat/qat_common/adf_isr.c
+++ b/drivers/crypto/qat/qat_common/adf_isr.c
@@ -15,6 +15,10 @@
#include "adf_transport_access_macros.h"
#include "adf_transport_internal.h"

+#ifdef CONFIG_PCI_IOV
+#define ADF_MAX_NUM_VFS 32
+#endif
+
static int adf_enable_msix(struct adf_accel_dev *accel_dev)
{
struct adf_accel_pci *pci_dev_info = &accel_dev->accel_pci_dev;
@@ -72,7 +76,7 @@ static irqreturn_t adf_msix_isr_ae(int irq, void *dev_ptr)
struct adf_bar *pmisc =
&GET_BARS(accel_dev)[hw_data->get_misc_bar_id(hw_data)];
void __iomem *pmisc_bar_addr = pmisc->virt_addr;
- u32 vf_mask;
+ unsigned long vf_mask;

/* Get the interrupt sources triggered by VFs */
vf_mask = ((ADF_CSR_RD(pmisc_bar_addr, ADF_ERRSOU5) &
@@ -93,8 +97,7 @@ static irqreturn_t adf_msix_isr_ae(int irq, void *dev_ptr)
* unless the VF is malicious and is attempting to
* flood the host OS with VF2PF interrupts.
*/
- for_each_set_bit(i, (const unsigned long *)&vf_mask,
- (sizeof(vf_mask) * BITS_PER_BYTE)) {
+ for_each_set_bit(i, &vf_mask, ADF_MAX_NUM_VFS) {
vf_info = accel_dev->pf.vf_info + i;

if (!__ratelimit(&vf_info->vf2pf_ratelimit)) {
--
2.26.2


2021-06-03 12:16:31

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 01/10] crypto: qat - use proper type for vf_mask

On Thu, May 27, 2021 at 08:12:42PM +0100, Marco Chiappero wrote:
>
> diff --git a/drivers/crypto/qat/qat_common/adf_isr.c b/drivers/crypto/qat/qat_common/adf_isr.c
> index e3ad5587be49..22f8ef5bfbc5 100644
> --- a/drivers/crypto/qat/qat_common/adf_isr.c
> +++ b/drivers/crypto/qat/qat_common/adf_isr.c
> @@ -15,6 +15,10 @@
> #include "adf_transport_access_macros.h"
> #include "adf_transport_internal.h"
>
> +#ifdef CONFIG_PCI_IOV
> +#define ADF_MAX_NUM_VFS 32
> +#endif

The #ifdef is not necessary.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-06-04 09:26:14

by Chiappero, Marco

[permalink] [raw]
Subject: RE: [PATCH 01/10] crypto: qat - use proper type for vf_mask

> -----Original Message-----
> From: Herbert Xu <[email protected]>
> Sent: Thursday, June 3, 2021 1:16 PM
> To: Chiappero, Marco <[email protected]>
> Cc: [email protected]; qat-linux <[email protected]>; Cabiddu,
> Giovanni <[email protected]>
> Subject: Re: [PATCH 01/10] crypto: qat - use proper type for vf_mask
>
> On Thu, May 27, 2021 at 08:12:42PM +0100, Marco Chiappero wrote:
> >
> > diff --git a/drivers/crypto/qat/qat_common/adf_isr.c
> > b/drivers/crypto/qat/qat_common/adf_isr.c
> > index e3ad5587be49..22f8ef5bfbc5 100644
> > --- a/drivers/crypto/qat/qat_common/adf_isr.c
> > +++ b/drivers/crypto/qat/qat_common/adf_isr.c
> > @@ -15,6 +15,10 @@
> > #include "adf_transport_access_macros.h"
> > #include "adf_transport_internal.h"
> >
> > +#ifdef CONFIG_PCI_IOV
> > +#define ADF_MAX_NUM_VFS 32
> > +#endif
>
> The #ifdef is not necessary.

Right, will resubmit soon.

Thank you,
Marco