* [PATCH] crypto: qat - Fix KASAN stack-out-of-bounds bug in adf_probe()
@ 2018-09-23 0:41 Waiman Long
2018-09-28 5:10 ` Herbert Xu
0 siblings, 1 reply; 2+ messages in thread
From: Waiman Long @ 2018-09-23 0:41 UTC (permalink / raw)
To: Giovanni Cabiddu, Herbert Xu, David S. Miller
Cc: qat-linux, linux-crypto, linux-kernel, Conor McLoughlin, Waiman Long
The following KASAN warning was printed when booting a 64-bit kernel
on some systems with Intel CPUs:
[ 44.512826] ==================================================================
[ 44.520165] BUG: KASAN: stack-out-of-bounds in find_first_bit+0xb0/0xc0
[ 44.526786] Read of size 8 at addr ffff88041e02fc50 by task kworker/0:2/124
[ 44.535253] CPU: 0 PID: 124 Comm: kworker/0:2 Tainted: G X --------- --- 4.18.0-12.el8.x86_64+debug #1
[ 44.545858] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS BKVDTRL1.86B.0005.D08.1712070559 12/07/2017
[ 44.555682] Workqueue: events work_for_cpu_fn
[ 44.560043] Call Trace:
[ 44.562502] dump_stack+0x9a/0xe9
[ 44.565832] print_address_description+0x65/0x22e
[ 44.570683] ? find_first_bit+0xb0/0xc0
[ 44.570689] kasan_report.cold.6+0x92/0x19f
[ 44.578726] find_first_bit+0xb0/0xc0
[ 44.578737] adf_probe+0x9eb/0x19a0 [qat_c62x]
[ 44.578751] ? adf_remove+0x110/0x110 [qat_c62x]
[ 44.591490] ? mark_held_locks+0xc8/0x140
[ 44.591498] ? _raw_spin_unlock+0x30/0x30
[ 44.591505] ? trace_hardirqs_on_caller+0x381/0x570
[ 44.604418] ? adf_remove+0x110/0x110 [qat_c62x]
[ 44.604427] local_pci_probe+0xd4/0x180
[ 44.604432] ? pci_device_shutdown+0x110/0x110
[ 44.617386] work_for_cpu_fn+0x51/0xa0
[ 44.621145] process_one_work+0x8fe/0x16e0
[ 44.625263] ? pwq_dec_nr_in_flight+0x2d0/0x2d0
[ 44.629799] ? lock_acquire+0x14c/0x400
[ 44.633645] ? move_linked_works+0x12e/0x2a0
[ 44.637928] worker_thread+0x536/0xb50
[ 44.641690] ? __kthread_parkme+0xb6/0x180
[ 44.645796] ? process_one_work+0x16e0/0x16e0
[ 44.650160] kthread+0x30c/0x3d0
[ 44.653400] ? kthread_create_worker_on_cpu+0xc0/0xc0
[ 44.658457] ret_from_fork+0x3a/0x50
[ 44.663557] The buggy address belongs to the page:
[ 44.668350] page:ffffea0010780bc0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
[ 44.676356] flags: 0x17ffffc0000000()
[ 44.680023] raw: 0017ffffc0000000 ffffea0010780bc8 ffffea0010780bc8 0000000000000000
[ 44.687769] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
[ 44.695510] page dumped because: kasan: bad access detected
[ 44.702578] Memory state around the buggy address:
[ 44.707372] ffff88041e02fb00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 44.714593] ffff88041e02fb80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 44.721810] >ffff88041e02fc00: 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 f2 f2 f2 f2
[ 44.729028] ^
[ 44.734864] ffff88041e02fc80: f2 f2 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00
[ 44.742082] ffff88041e02fd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 44.749299] ==================================================================
Looking into the code:
int ret, bar_mask;
:
for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
It is casting a 32-bit integer pointer to a 64-bit unsigned long
pointer. There are two problems here. First, the 32-bit pointer address
may not be 64-bit aligned. Secondly, it is accessing an extra 4 bytes.
This is fixed by changing the bar_mask type to unsigned long.
Signed-off-by: Waiman Long <longman@redhat.com>
---
drivers/crypto/qat/qat_c3xxx/adf_drv.c | 6 +++---
drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 6 +++---
drivers/crypto/qat/qat_c62x/adf_drv.c | 6 +++---
drivers/crypto/qat/qat_c62xvf/adf_drv.c | 6 +++---
drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 6 +++---
drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 6 +++---
6 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/crypto/qat/qat_c3xxx/adf_drv.c b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
index ba197f3..763c216 100644
--- a/drivers/crypto/qat/qat_c3xxx/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
@@ -123,7 +123,8 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct adf_hw_device_data *hw_data;
char name[ADF_DEVICE_NAME_LENGTH];
unsigned int i, bar_nr;
- int ret, bar_mask;
+ unsigned long bar_mask;
+ int ret;
switch (ent->device) {
case ADF_C3XXX_PCI_DEVICE_ID:
@@ -235,8 +236,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Find and map all the device's BARS */
i = 0;
bar_mask = pci_select_bars(pdev, IORESOURCE_MEM);
- for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
- ADF_PCI_MAX_BARS * 2) {
+ for_each_set_bit(bar_nr, &bar_mask, ADF_PCI_MAX_BARS * 2) {
struct adf_bar *bar = &accel_pci_dev->pci_bars[i++];
bar->base_addr = pci_resource_start(pdev, bar_nr);
diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
index 24ec908..613c7d5 100644
--- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
@@ -125,7 +125,8 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct adf_hw_device_data *hw_data;
char name[ADF_DEVICE_NAME_LENGTH];
unsigned int i, bar_nr;
- int ret, bar_mask;
+ unsigned long bar_mask;
+ int ret;
switch (ent->device) {
case ADF_C3XXXIOV_PCI_DEVICE_ID:
@@ -215,8 +216,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Find and map all the device's BARS */
i = 0;
bar_mask = pci_select_bars(pdev, IORESOURCE_MEM);
- for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
- ADF_PCI_MAX_BARS * 2) {
+ for_each_set_bit(bar_nr, &bar_mask, ADF_PCI_MAX_BARS * 2) {
struct adf_bar *bar = &accel_pci_dev->pci_bars[i++];
bar->base_addr = pci_resource_start(pdev, bar_nr);
diff --git a/drivers/crypto/qat/qat_c62x/adf_drv.c b/drivers/crypto/qat/qat_c62x/adf_drv.c
index 59a5a0d..9cb8329 100644
--- a/drivers/crypto/qat/qat_c62x/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62x/adf_drv.c
@@ -123,7 +123,8 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct adf_hw_device_data *hw_data;
char name[ADF_DEVICE_NAME_LENGTH];
unsigned int i, bar_nr;
- int ret, bar_mask;
+ unsigned long bar_mask;
+ int ret;
switch (ent->device) {
case ADF_C62X_PCI_DEVICE_ID:
@@ -235,8 +236,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Find and map all the device's BARS */
i = (hw_data->fuses & ADF_DEVICE_FUSECTL_MASK) ? 1 : 0;
bar_mask = pci_select_bars(pdev, IORESOURCE_MEM);
- for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
- ADF_PCI_MAX_BARS * 2) {
+ for_each_set_bit(bar_nr, &bar_mask, ADF_PCI_MAX_BARS * 2) {
struct adf_bar *bar = &accel_pci_dev->pci_bars[i++];
bar->base_addr = pci_resource_start(pdev, bar_nr);
diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c
index b9f3e0e..278452b 100644
--- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c
@@ -125,7 +125,8 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct adf_hw_device_data *hw_data;
char name[ADF_DEVICE_NAME_LENGTH];
unsigned int i, bar_nr;
- int ret, bar_mask;
+ unsigned long bar_mask;
+ int ret;
switch (ent->device) {
case ADF_C62XIOV_PCI_DEVICE_ID:
@@ -215,8 +216,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Find and map all the device's BARS */
i = 0;
bar_mask = pci_select_bars(pdev, IORESOURCE_MEM);
- for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
- ADF_PCI_MAX_BARS * 2) {
+ for_each_set_bit(bar_nr, &bar_mask, ADF_PCI_MAX_BARS * 2) {
struct adf_bar *bar = &accel_pci_dev->pci_bars[i++];
bar->base_addr = pci_resource_start(pdev, bar_nr);
diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
index be5c5a9..3a9708e 100644
--- a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
@@ -123,7 +123,8 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct adf_hw_device_data *hw_data;
char name[ADF_DEVICE_NAME_LENGTH];
unsigned int i, bar_nr;
- int ret, bar_mask;
+ unsigned long bar_mask;
+ int ret;
switch (ent->device) {
case ADF_DH895XCC_PCI_DEVICE_ID:
@@ -237,8 +238,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Find and map all the device's BARS */
i = 0;
bar_mask = pci_select_bars(pdev, IORESOURCE_MEM);
- for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
- ADF_PCI_MAX_BARS * 2) {
+ for_each_set_bit(bar_nr, &bar_mask, ADF_PCI_MAX_BARS * 2) {
struct adf_bar *bar = &accel_pci_dev->pci_bars[i++];
bar->base_addr = pci_resource_start(pdev, bar_nr);
diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
index 26ab17b..3da0f95 100644
--- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
@@ -125,7 +125,8 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct adf_hw_device_data *hw_data;
char name[ADF_DEVICE_NAME_LENGTH];
unsigned int i, bar_nr;
- int ret, bar_mask;
+ unsigned long bar_mask;
+ int ret;
switch (ent->device) {
case ADF_DH895XCCIOV_PCI_DEVICE_ID:
@@ -215,8 +216,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Find and map all the device's BARS */
i = 0;
bar_mask = pci_select_bars(pdev, IORESOURCE_MEM);
- for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
- ADF_PCI_MAX_BARS * 2) {
+ for_each_set_bit(bar_nr, &bar_mask, ADF_PCI_MAX_BARS * 2) {
struct adf_bar *bar = &accel_pci_dev->pci_bars[i++];
bar->base_addr = pci_resource_start(pdev, bar_nr);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] crypto: qat - Fix KASAN stack-out-of-bounds bug in adf_probe()
2018-09-23 0:41 [PATCH] crypto: qat - Fix KASAN stack-out-of-bounds bug in adf_probe() Waiman Long
@ 2018-09-28 5:10 ` Herbert Xu
0 siblings, 0 replies; 2+ messages in thread
From: Herbert Xu @ 2018-09-28 5:10 UTC (permalink / raw)
To: Waiman Long
Cc: Giovanni Cabiddu, David S. Miller, qat-linux, linux-crypto,
linux-kernel, Conor McLoughlin
On Sat, Sep 22, 2018 at 08:41:55PM -0400, Waiman Long wrote:
> The following KASAN warning was printed when booting a 64-bit kernel
> on some systems with Intel CPUs:
>
> [ 44.512826] ==================================================================
> [ 44.520165] BUG: KASAN: stack-out-of-bounds in find_first_bit+0xb0/0xc0
> [ 44.526786] Read of size 8 at addr ffff88041e02fc50 by task kworker/0:2/124
>
> [ 44.535253] CPU: 0 PID: 124 Comm: kworker/0:2 Tainted: G X --------- --- 4.18.0-12.el8.x86_64+debug #1
> [ 44.545858] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS BKVDTRL1.86B.0005.D08.1712070559 12/07/2017
> [ 44.555682] Workqueue: events work_for_cpu_fn
> [ 44.560043] Call Trace:
> [ 44.562502] dump_stack+0x9a/0xe9
> [ 44.565832] print_address_description+0x65/0x22e
> [ 44.570683] ? find_first_bit+0xb0/0xc0
> [ 44.570689] kasan_report.cold.6+0x92/0x19f
> [ 44.578726] find_first_bit+0xb0/0xc0
> [ 44.578737] adf_probe+0x9eb/0x19a0 [qat_c62x]
> [ 44.578751] ? adf_remove+0x110/0x110 [qat_c62x]
> [ 44.591490] ? mark_held_locks+0xc8/0x140
> [ 44.591498] ? _raw_spin_unlock+0x30/0x30
> [ 44.591505] ? trace_hardirqs_on_caller+0x381/0x570
> [ 44.604418] ? adf_remove+0x110/0x110 [qat_c62x]
> [ 44.604427] local_pci_probe+0xd4/0x180
> [ 44.604432] ? pci_device_shutdown+0x110/0x110
> [ 44.617386] work_for_cpu_fn+0x51/0xa0
> [ 44.621145] process_one_work+0x8fe/0x16e0
> [ 44.625263] ? pwq_dec_nr_in_flight+0x2d0/0x2d0
> [ 44.629799] ? lock_acquire+0x14c/0x400
> [ 44.633645] ? move_linked_works+0x12e/0x2a0
> [ 44.637928] worker_thread+0x536/0xb50
> [ 44.641690] ? __kthread_parkme+0xb6/0x180
> [ 44.645796] ? process_one_work+0x16e0/0x16e0
> [ 44.650160] kthread+0x30c/0x3d0
> [ 44.653400] ? kthread_create_worker_on_cpu+0xc0/0xc0
> [ 44.658457] ret_from_fork+0x3a/0x50
>
> [ 44.663557] The buggy address belongs to the page:
> [ 44.668350] page:ffffea0010780bc0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
> [ 44.676356] flags: 0x17ffffc0000000()
> [ 44.680023] raw: 0017ffffc0000000 ffffea0010780bc8 ffffea0010780bc8 0000000000000000
> [ 44.687769] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> [ 44.695510] page dumped because: kasan: bad access detected
>
> [ 44.702578] Memory state around the buggy address:
> [ 44.707372] ffff88041e02fb00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 44.714593] ffff88041e02fb80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 44.721810] >ffff88041e02fc00: 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 f2 f2 f2 f2
> [ 44.729028] ^
> [ 44.734864] ffff88041e02fc80: f2 f2 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00
> [ 44.742082] ffff88041e02fd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 44.749299] ==================================================================
>
> Looking into the code:
>
> int ret, bar_mask;
> :
> for_each_set_bit(bar_nr, (const unsigned long *)&bar_mask,
>
> It is casting a 32-bit integer pointer to a 64-bit unsigned long
> pointer. There are two problems here. First, the 32-bit pointer address
> may not be 64-bit aligned. Secondly, it is accessing an extra 4 bytes.
>
> This is fixed by changing the bar_mask type to unsigned long.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
Patch 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] 2+ messages in thread
end of thread, other threads:[~2018-09-28 5:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-23 0:41 [PATCH] crypto: qat - Fix KASAN stack-out-of-bounds bug in adf_probe() Waiman Long
2018-09-28 5:10 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).