[RFC RESEND 0/2] megaraid: fix illegal pointer dereference in complete_cmd_fusion

[RFC RESEND 0/2] megaraid: fix illegal pointer dereference in complete_cmd_fusion

[chengkaitao]

From: Chengkaitao <chengkaitao@kylinos.cn>

We have encountered several instances of null pointer dereferences
in the complete_cmd_fusion function. The root cause was consistently
the reply_desc->SMID being erroneously set to 0xffff, resulting in
an invalid cmd_fusion pointer retrieval.

After code review, I suspect synchronization issues in complete_cmd-
_fusion. However, due to the low reproducibility rate of this problem,
I could not perform effective validation. I propose the patch for
kernel community discussion to explore better solutions.

Chengkaitao (2):
  megaraid: Fix the issue of erroneous reset of Words in reply_desc
  megaraid: replacing fusion->busy_mq_poll[*] with irq_context->in_used
    in megasas_blk_mq_poll

 drivers/scsi/megaraid/megaraid_sas_fusion.c | 18 ++++++++++--------
 drivers/scsi/megaraid/megaraid_sas_fusion.h |  2 --
 2 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.50.1 (Apple Git-155)

[RFC RESEND 1/2] megaraid: Fix the issue of erroneous reset of Words in reply_desc

[chengkaitao]

From: Chengkaitao <chengkaitao@kylinos.cn>

The following panic occurred on kernel 6.6 (arch: loongarch):
Call Trace:
complete_cmd_fusion+0x180/0x7c8 [megaraid_sas]
megasas_isr_fusion+0xd4/0xf0 [megaraid_sas]
__handle_irq_event_percpu+0x70/0x228
handle_irq_event+0x44/0xf8
handle_edge_irq+0xe8/0x328
avecintc_irq_dispatch+0x68/0x120
handle_irq_desc+0x5c/0x78
handle_cpu_irq+0x6c/0xa8
handle_loongarch_irq+0x2c/0x48
do_vint+0x7c/0xd0
sched_update_worker+0x8/0x90
worker_thread+0x218/0x480
kthread+0xf0/0xf8
ret_from_kernel_thread+0x28/0xc8
ret_from_kernel_thread_asm+0xc/0xa0

Observed symptoms during the issue:
complete_cmd_fusion(struct megasas_instance *instance, u32 MSIxIndex,
		    struct megasas_irq_context *irq_context)
{
	******

	while (d_val.u.low != cpu_to_le32(UINT_MAX) &&
	       d_val.u.high != cpu_to_le32(UINT_MAX)) {

	/**
		When the issue occurs:
		d_val.u.low == 60293120
		d_val.u.high == 0
		reply_desc->SMID == 0xffff
 	 **/
		smid = le16_to_cpu(reply_desc->SMID);
		cmd_fusion = fusion->cmd_list[smid - 1];
		scsi_io_req = (struct MPI2_RAID_SCSI_IO_REQUEST *)
						cmd_fusion->io_request;
	/** cmd_fusion becomes an invalid pointer **/

	******
}

In the complete_cmd_fusion function, the following assignment exists:
	d_val.word = desc->Words;
Thus, reply_desc->SMID == 0xffff may be caused by a concurrency-related
corruption.

Reproduction probability is very low. After code review, I suspect the
following race condition scenario:

interrupt(complete_cmd_fusion)           cpu1(megasas_reset_reply_desc)
while (d_val.u.low != *****) {
        scsi_io_req =
            cmd_fusion->io_request;

        d_val.word = desc->Words;
                                        reply_desc->Words =
                                          cpu_to_le64(ULLONG_MAX);
}

Note: This is a proposed patch for discussion only. It has not been
verified to resolve the issue. If you have alternative suggestions,
please join the discussion.

Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
Reported-by: Zheng tan <tanzheng@kylinos.cn>
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index a6794f49e9fa..3d3480b19734 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -4282,16 +4282,23 @@ void  megasas_reset_reply_desc(struct megasas_instance *instance)
 	int i, j, count;
 	struct fusion_context *fusion;
 	union MPI2_REPLY_DESCRIPTORS_UNION *reply_desc;
+	struct megasas_irq_context *irq_context;
 
 	fusion = instance->ctrl_context;
 	count = instance->msix_vectors > 0 ? instance->msix_vectors : 1;
 	count += instance->iopoll_q_count;
 
 	for (i = 0 ; i < count ; i++) {
+		irq_context = &instance->irq_context[i];
+		while (!access_irq_context(irq_context))
+			cpu_relax();
+
 		fusion->last_reply_idx[i] = 0;
 		reply_desc = fusion->reply_frames_desc[i];
 		for (j = 0 ; j < fusion->reply_q_depth; j++, reply_desc++)
 			reply_desc->Words = cpu_to_le64(ULLONG_MAX);
+
+		release_irq_context(irq_context);
 	}
 }
 
-- 
2.50.1 (Apple Git-155)

[RFC RESEND 2/2] megaraid: replacing fusion->busy_mq_poll[*] with irq_context->in_used in megasas_blk_mq_poll

[chengkaitao]

From: Chengkaitao <chengkaitao@kylinos.cn>

The following two types of kernel panics occur on the 4.19 kernel:
Call Trace:
complete_cmd_fusion+0x448/0x6a0 [megaraid_sas]
megasas_blk_mq_poll+0xa8/0x110 [megaraid_sas]
scsi_mq_poll+0x38/0x50
blk_mq_poll+0x198/0x2d8
blk_poll+0x60/0x70
swap_readpage+0x1b0/0x260
read_swap_cache_async+0x5c/0x78
swap_cluster_readahead+0x1e0/0x2b0
swapin_readahead+0x100/0x4c0
do_swap_page+0x244/0xb40
__handle_mm_fault+0x4b0/0x560
handle_mm_fault+0x114/0x280
do_page_fault+0x1f8/0x4c0
do_translation_fault+0xa8/0xbc
do_mem_abort+0x50/0xe0
el1_da+0x20/0x94

Call trace:
complete_cmd_fusion+0x448/0x6a0 [megaraid_sas]
megasas_isr_fusion+0x98/0xa8 [megaraid_sas]
__handle_irq_event_percpu+0x64/0x260
handle_irq_event_percpu+0x28/0x60
handle_irq_event+0x50/0xf8
handle_fasteoi_edge_irq+0x190/0x208
generic_handle_irq+0x3c/0x58
__handle_domain_irq+0x68/0xc0
gic_handle_irq+0x78/0x180
el1_irq+0xb8/0x140

Later, we applied commit 9650b453a3d4 ("block: ignore RWF_HIPRI hint
for sync dio"), and the issue disappeared. Although most of the mq-poll
paths have been removed upstream, io-uring related calls still remain.

We cannot completely rule out the possibility of [patch 1/2] causing
the issue. I still suspect a concurrency/race condition between
megasas_blk_mq_poll and megasas_isr_fusion. Although historical patch
commit logs mention that interrupts are disabled when polling is used,
I haven't found code evidence to confirm it.

Replacing fusion->busy_mq_poll[*] with irq_context->in_used serves two
purposes:
To handle synchronization issues between mq-poll and megasas_isr_fusion
To handle synchronization between mq-poll and megasas_reset_reply_desc

Note: This is a proposed patch for discussion only. It has not been
verified to resolve the issue. If you have alternative suggestions,
please join the discussion.

Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
Reported-by: Zheng tan <tanzheng@kylinos.cn>
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 11 +++--------
 drivers/scsi/megaraid/megaraid_sas_fusion.h |  2 --
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 3d3480b19734..b647bec7115b 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1871,9 +1871,6 @@ megasas_init_adapter_fusion(struct megasas_instance *instance)
 				MEGASAS_FUSION_IOCTL_CMDS);
 	sema_init(&instance->ioctl_sem, MEGASAS_FUSION_IOCTL_CMDS);
 
-	for (i = 0; i < MAX_MSIX_QUEUES_FUSION; i++)
-		atomic_set(&fusion->busy_mq_poll[i], 0);
-
 	if (megasas_alloc_ioc_init_frame(instance))
 		return 1;
 
@@ -3731,6 +3728,7 @@ int megasas_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 	struct megasas_instance *instance;
 	int num_entries = 0;
 	struct fusion_context *fusion;
+	struct megasas_irq_context *irq_context;
 
 	instance = (struct megasas_instance *)shost->hostdata;
 
@@ -3738,11 +3736,8 @@ int megasas_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 
 	queue_num = queue_num + instance->low_latency_index_start;
 
-	if (!atomic_add_unless(&fusion->busy_mq_poll[queue_num], 1, 1))
-		return 0;
-
-	num_entries = complete_cmd_fusion(instance, queue_num, NULL);
-	atomic_dec(&fusion->busy_mq_poll[queue_num]);
+	irq_context = &instance->irq_context[queue_num];
+	num_entries = complete_cmd_fusion(instance, queue_num, irq_context);
 
 	return num_entries;
 }
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h b/drivers/scsi/megaraid/megaraid_sas_fusion.h
index ddeea0ee2834..70679f53bf9d 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
@@ -1313,8 +1313,6 @@ struct fusion_context {
 	u8 *sense;
 	dma_addr_t sense_phys_addr;
 
-	atomic_t   busy_mq_poll[MAX_MSIX_QUEUES_FUSION];
-
 	dma_addr_t reply_frames_desc_phys[MAX_MSIX_QUEUES_FUSION];
 	union MPI2_REPLY_DESCRIPTORS_UNION *reply_frames_desc[MAX_MSIX_QUEUES_FUSION];
 	struct rdpq_alloc_detail rdpq_tracker[RDPQ_MAX_CHUNK_COUNT];
-- 
2.50.1 (Apple Git-155)