Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization
> "Calvin" == Calvin Owens writes: Calvin> In _base_make_ioc_operational(), we walk ioc->reply_queue_list Calvin> and pull a pointer out of successive elements of Calvin> ioc->reply_post[] for each entry in that list if RDPQ is Calvin> enabled. Calvin> Since the code pulls the pointer for the next iteration at the Calvin> bottom of the loop, it triggers the a KASAN dump on the final Calvin> iteration: Applied to 4.6/scsi-fixes. Thanks! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization
Hi, Please consider this patch as Ack-by: Chaitra P B Thanks, Chaitra -Original Message- From: Martin K. Petersen [mailto:martin.peter...@oracle.com] Sent: Tuesday, March 22, 2016 6:00 AM To: Calvin Owens Cc: Sathya Prakash; Chaitra P B; Suganath Prabu Subramani; James E.J. Bottomley; Martin K. Petersen; mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; kernel-t...@fb.com; Sreekanth Reddy Subject: Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization >>>>> "Calvin" == Calvin Owens writes: Calvin> In _base_make_ioc_operational(), we walk ioc->reply_queue_list Calvin> and pull a pointer out of successive elements of Calvin> ioc->reply_post[] for each entry in that list if RDPQ is Calvin> enabled. Calvin> Since the code pulls the pointer for the next iteration at the Calvin> bottom of the loop, it triggers the a KASAN dump on the final Calvin> iteration: Broadcom folks, please review. Thanks! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization
On 03/18/2016 01:45 PM, Calvin Owens wrote: In _base_make_ioc_operational(), we walk ioc->reply_queue_list and pull a pointer out of successive elements of ioc->reply_post[] for each entry in that list if RDPQ is enabled. Since the code pulls the pointer for the next iteration at the bottom of the loop, it triggers the a KASAN dump on the final iteration: BUG: KASAN: slab-out-of-bounds in _base_make_ioc_operational+0x47b7/0x47e0 [mpt3sas] at addr 880754816ab0 Read of size 8 by task modprobe/305 Call Trace: [] dump_stack+0x4d/0x6c [] print_trailer+0xf9/0x150 [] object_err+0x34/0x40 [] kasan_report_error+0x221/0x530 [] __asan_report_load8_noabort+0x43/0x50 [] _base_make_ioc_operational+0x47b7/0x47e0 [mpt3sas] [] mpt3sas_base_attach+0x1991/0x2120 [mpt3sas] [] _scsih_probe+0xeb3/0x16b0 [mpt3sas] [] local_pci_probe+0xc7/0x170 [] pci_device_probe+0x20f/0x290 [] really_probe+0x17d/0x600 [] __driver_attach+0x153/0x190 [] bus_for_each_dev+0x11c/0x1a0 [] driver_attach+0x3d/0x50 [] bus_add_driver+0x44a/0x5f0 [] driver_register+0x18c/0x3b0 [] __pci_register_driver+0x156/0x200 [] _mpt3sas_init+0x135/0x1000 [mpt3sas] [] do_one_initcall+0x113/0x2b0 [] do_init_module+0x1d0/0x4d8 [] load_module+0x6729/0x8dc0 [] SYSC_init_module+0x183/0x1a0 [] SyS_init_module+0xe/0x10 [] entry_SYSCALL_64_fastpath+0x12/0x6a Fix this by pulling the value at the beginning of the loop. Signed-off-by: Calvin Owens Looks good to me. Reviewed-by: Jens Axboe -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization
Martin, This patch is being reviewed , we shall get back with reviews by tomorrow. Thanks, Chaitra -Original Message- From: Martin K. Petersen [mailto:martin.peter...@oracle.com] Sent: Tuesday, March 22, 2016 6:00 AM To: Calvin Owens Cc: Sathya Prakash; Chaitra P B; Suganath Prabu Subramani; James E.J. Bottomley; Martin K. Petersen; mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; kernel-t...@fb.com; Sreekanth Reddy Subject: Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization >>>>> "Calvin" == Calvin Owens writes: Calvin> In _base_make_ioc_operational(), we walk ioc->reply_queue_list Calvin> and pull a pointer out of successive elements of Calvin> ioc->reply_post[] for each entry in that list if RDPQ is Calvin> enabled. Calvin> Since the code pulls the pointer for the next iteration at the Calvin> bottom of the loop, it triggers the a KASAN dump on the final Calvin> iteration: Broadcom folks, please review. Thanks! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization
> "Calvin" == Calvin Owens writes: Calvin> In _base_make_ioc_operational(), we walk ioc->reply_queue_list Calvin> and pull a pointer out of successive elements of Calvin> ioc->reply_post[] for each entry in that list if RDPQ is Calvin> enabled. Calvin> Since the code pulls the pointer for the next iteration at the Calvin> bottom of the loop, it triggers the a KASAN dump on the final Calvin> iteration: Broadcom folks, please review. Thanks! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization
On Freitag, 18. März 2016 12:45:42 CET Calvin Owens wrote: > In _base_make_ioc_operational(), we walk ioc->reply_queue_list and pull > a pointer out of successive elements of ioc->reply_post[] for each entry > in that list if RDPQ is enabled. > > Since the code pulls the pointer for the next iteration at the bottom of > the loop, it triggers the a KASAN dump on the final iteration: > > BUG: KASAN: slab-out-of-bounds in > _base_make_ioc_operational+0x47b7/0x47e0 [mpt3sas] at addr 880754816ab0 > Read of size 8 by task modprobe/305 > > Call Trace: > [] dump_stack+0x4d/0x6c > [] print_trailer+0xf9/0x150 > [] object_err+0x34/0x40 > [] kasan_report_error+0x221/0x530 > [] __asan_report_load8_noabort+0x43/0x50 > [] _base_make_ioc_operational+0x47b7/0x47e0 [mpt3sas] > [] mpt3sas_base_attach+0x1991/0x2120 [mpt3sas] > [] _scsih_probe+0xeb3/0x16b0 [mpt3sas] > [] local_pci_probe+0xc7/0x170 > [] pci_device_probe+0x20f/0x290 > [] really_probe+0x17d/0x600 > [] __driver_attach+0x153/0x190 > [] bus_for_each_dev+0x11c/0x1a0 > [] driver_attach+0x3d/0x50 > [] bus_add_driver+0x44a/0x5f0 > [] driver_register+0x18c/0x3b0 > [] __pci_register_driver+0x156/0x200 > [] _mpt3sas_init+0x135/0x1000 [mpt3sas] > [] do_one_initcall+0x113/0x2b0 > [] do_init_module+0x1d0/0x4d8 > [] load_module+0x6729/0x8dc0 > [] SYSC_init_module+0x183/0x1a0 > [] SyS_init_module+0xe/0x10 > [] entry_SYSCALL_64_fastpath+0x12/0x6a > > Fix this by pulling the value at the beginning of the loop. > > Signed-off-by: Calvin Owens Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization
In _base_make_ioc_operational(), we walk ioc->reply_queue_list and pull
a pointer out of successive elements of ioc->reply_post[] for each entry
in that list if RDPQ is enabled.
Since the code pulls the pointer for the next iteration at the bottom of
the loop, it triggers the a KASAN dump on the final iteration:
BUG: KASAN: slab-out-of-bounds in _base_make_ioc_operational+0x47b7/0x47e0
[mpt3sas] at addr 880754816ab0
Read of size 8 by task modprobe/305
Call Trace:
[] dump_stack+0x4d/0x6c
[] print_trailer+0xf9/0x150
[] object_err+0x34/0x40
[] kasan_report_error+0x221/0x530
[] __asan_report_load8_noabort+0x43/0x50
[] _base_make_ioc_operational+0x47b7/0x47e0 [mpt3sas]
[] mpt3sas_base_attach+0x1991/0x2120 [mpt3sas]
[] _scsih_probe+0xeb3/0x16b0 [mpt3sas]
[] local_pci_probe+0xc7/0x170
[] pci_device_probe+0x20f/0x290
[] really_probe+0x17d/0x600
[] __driver_attach+0x153/0x190
[] bus_for_each_dev+0x11c/0x1a0
[] driver_attach+0x3d/0x50
[] bus_add_driver+0x44a/0x5f0
[] driver_register+0x18c/0x3b0
[] __pci_register_driver+0x156/0x200
[] _mpt3sas_init+0x135/0x1000 [mpt3sas]
[] do_one_initcall+0x113/0x2b0
[] do_init_module+0x1d0/0x4d8
[] load_module+0x6729/0x8dc0
[] SYSC_init_module+0x183/0x1a0
[] SyS_init_module+0xe/0x10
[] entry_SYSCALL_64_fastpath+0x12/0x6a
Fix this by pulling the value at the beginning of the loop.
Signed-off-by: Calvin Owens
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index e4db5fb..8c44b9c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -5030,7 +5030,7 @@ _base_make_ioc_ready(struct MPT3SAS_ADAPTER *ioc, int
sleep_flag,
static int
_base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
{
- int r, i;
+ int r, i, index;
unsigned long flags;
u32 reply_address;
u16 smid;
@@ -5039,8 +5039,7 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc,
int sleep_flag)
struct _event_ack_list *delayed_event_ack, *delayed_event_ack_next;
u8 hide_flag;
struct adapter_reply_queue *reply_q;
- long reply_post_free;
- u32 reply_post_free_sz, index = 0;
+ Mpi2ReplyDescriptorsUnion_t *reply_post_free_contig;
dinitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
__func__));
@@ -5124,27 +5123,27 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc,
int sleep_flag)
_base_assign_reply_queues(ioc);
/* initialize Reply Post Free Queue */
- reply_post_free_sz = ioc->reply_post_queue_depth *
- sizeof(Mpi2DefaultReplyDescriptor_t);
- reply_post_free = (long)ioc->reply_post[index].reply_post_free;
+ index = 0;
+ reply_post_free_contig = ioc->reply_post[0].reply_post_free;
list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+ /*
+* If RDPQ is enabled, switch to the next allocation.
+* Otherwise advance within the contiguous region.
+*/
+ if (ioc->rdpq_array_enable) {
+ reply_q->reply_post_free =
+ ioc->reply_post[index++].reply_post_free;
+ } else {
+ reply_q->reply_post_free = reply_post_free_contig;
+ reply_post_free_contig += ioc->reply_post_queue_depth;
+ }
+
reply_q->reply_post_host_index = 0;
- reply_q->reply_post_free = (Mpi2ReplyDescriptorsUnion_t *)
- reply_post_free;
for (i = 0; i < ioc->reply_post_queue_depth; i++)
reply_q->reply_post_free[i].Words =
cpu_to_le64(ULLONG_MAX);
if (!_base_is_controller_msix_enabled(ioc))
goto skip_init_reply_post_free_queue;
- /*
-* If RDPQ is enabled, switch to the next allocation.
-* Otherwise advance within the contiguous region.
-*/
- if (ioc->rdpq_array_enable)
- reply_post_free = (long)
- ioc->reply_post[++index].reply_post_free;
- else
- reply_post_free += reply_post_free_sz;
}
skip_init_reply_post_free_queue:
--
2.8.0.rc2
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
