diff mbox series

qla2xxx: Fix NULL pointer dereference in target mode

Message ID 32b0bb9f-ba6a-e9f1-e779-5af2e115c67a@scst.dev
State Superseded
Headers show
Series qla2xxx: Fix NULL pointer dereference in target mode | expand

Commit Message

Gleb Chesnokov May 11, 2023, 10:02 a.m. UTC
When target mode is enabled, the pci_irq_get_affinity() function may return
a NULL value in qla_mapq_init_qp_cpu_map() due to the qla24xx_enable_msix()
code that handles IRQ settings for target mode. This leads to a crash due
to a NULL pointer dereference.

This patch fixes the issue by adding a check for the NULL value returned
by pci_irq_get_affinity() and introducing a 'cpu_mapped' boolean flag to
the qla_qpair structure, ensuring that the qpair's CPU affinity is updated
when it has not been mapped to a CPU.

Fixes: 1d201c81d4cc ("scsi: qla2xxx: Select qpair depending on which CPU 
post_cmd() gets called")

Signed-off-by: Gleb Chesnokov <gleb.chesnokov@scst.dev>
---
  drivers/scsi/qla2xxx/qla_def.h    | 1 +
  drivers/scsi/qla2xxx/qla_init.c   | 3 +++
  drivers/scsi/qla2xxx/qla_inline.h | 6 ++++++
  drivers/scsi/qla2xxx/qla_isr.c    | 3 +++
  4 files changed, 13 insertions(+)


  #define __update_rsp_in(_is_shadow_hba, _rsp, _rsp_in)            \

Comments

Himanshu Madhani May 12, 2023, 4:29 p.m. UTC | #1
> On May 11, 2023, at 3:02 AM, Gleb Chesnokov <gleb.chesnokov@scst.dev> wrote:
> 
> When target mode is enabled, the pci_irq_get_affinity() function may return
> a NULL value in qla_mapq_init_qp_cpu_map() due to the qla24xx_enable_msix()
> code that handles IRQ settings for target mode. This leads to a crash due
> to a NULL pointer dereference.
> 
> This patch fixes the issue by adding a check for the NULL value returned
> by pci_irq_get_affinity() and introducing a 'cpu_mapped' boolean flag to
> the qla_qpair structure, ensuring that the qpair's CPU affinity is updated
> when it has not been mapped to a CPU.
> 
> Fixes: 1d201c81d4cc ("scsi: qla2xxx: Select qpair depending on which CPU post_cmd() gets called")
> 
> Signed-off-by: Gleb Chesnokov <gleb.chesnokov@scst.dev>
> ---
>  drivers/scsi/qla2xxx/qla_def.h    | 1 +
>  drivers/scsi/qla2xxx/qla_init.c   | 3 +++
>  drivers/scsi/qla2xxx/qla_inline.h | 6 ++++++
>  drivers/scsi/qla2xxx/qla_isr.c    | 3 +++
>  4 files changed, 13 insertions(+)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index df5e5b7fdcfe..84aa3571be6d 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -3796,6 +3796,7 @@ struct qla_qpair {
>      uint64_t retry_term_jiff;
>      struct qla_tgt_counters tgt_counters;
>      uint16_t cpuid;
> +    bool cpu_mapped;
>      struct qla_fw_resources fwres ____cacheline_aligned;
>      struct  qla_buf_pool buf_pool;
>      u32    cmd_cnt;
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index ec0423ec6681..1a955c3ff3d6 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -9426,6 +9426,9 @@ struct qla_qpair *qla2xxx_create_qpair(struct scsi_qla_host *vha, int qos,
>          qpair->rsp->req = qpair->req;
>          qpair->rsp->qpair = qpair;
> 
> +        if (!qpair->cpu_mapped)
> +            qla_cpu_update(qpair, raw_smp_processor_id());
> +
>          if (IS_T10_PI_CAPABLE(ha) && ql2xenabledif) {
>              if (ha->fw_attributes & BIT_4)
>                  qpair->difdix_supported = 1;
> diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h
> index cce6e425c121..a6e2c7d4ff87 100644
> --- a/drivers/scsi/qla2xxx/qla_inline.h
> +++ b/drivers/scsi/qla2xxx/qla_inline.h
> @@ -538,12 +538,18 @@ qla_mapq_init_qp_cpu_map(struct qla_hw_data *ha,
> 
>      if (!ha->qp_cpu_map)
>          return;
> +
>      mask = pci_irq_get_affinity(ha->pdev, msix->vector_base0);
> +    if (!mask)
> +        return;
> +
>      qpair->cpuid = cpumask_first(mask);
>      for_each_cpu(cpu, mask) {
>          ha->qp_cpu_map[cpu] = qpair;
>      }
>      msix->cpuid = qpair->cpuid;
> +
> +    qpair->cpu_mapped = true;
>  }
> 
>  static inline void
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 71feda2cdb63..245e3a5d81fd 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -3770,6 +3770,9 @@ void qla24xx_process_response_queue(struct scsi_qla_host *vha,
> 
>      if (rsp->qpair->cpuid != smp_processor_id() || !rsp->qpair->rcv_intr) {
>          rsp->qpair->rcv_intr = 1;
> +
> +        if (!rsp->qpair->cpu_mapped)
> +            qla_cpu_update(rsp->qpair, raw_smp_processor_id());
>      }
> 
>  #define __update_rsp_in(_is_shadow_hba, _rsp, _rsp_in)            \
> -- 
> 2.40.1

Looks Good. 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com <mailto:himanshu.madhani@oracle.com>>
Martin K. Petersen May 17, 2023, 1:21 a.m. UTC | #2
Hi Gleb!

> When target mode is enabled, the pci_irq_get_affinity() function may
> return a NULL value in qla_mapq_init_qp_cpu_map() due to the
> qla24xx_enable_msix() code that handles IRQ settings for target mode.
> This leads to a crash due to a NULL pointer dereference.
>
> This patch fixes the issue by adding a check for the NULL value
> returned by pci_irq_get_affinity() and introducing a 'cpu_mapped'
> boolean flag to the qla_qpair structure, ensuring that the qpair's CPU
> affinity is updated when it has not been mapped to a CPU.

Your patch is whitespace-mangled. Please fix and resubmit.
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index df5e5b7fdcfe..84aa3571be6d 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3796,6 +3796,7 @@  struct qla_qpair {
      uint64_t retry_term_jiff;
      struct qla_tgt_counters tgt_counters;
      uint16_t cpuid;
+    bool cpu_mapped;
      struct qla_fw_resources fwres ____cacheline_aligned;
      struct  qla_buf_pool buf_pool;
      u32    cmd_cnt;
diff --git a/drivers/scsi/qla2xxx/qla_init.c 
b/drivers/scsi/qla2xxx/qla_init.c
index ec0423ec6681..1a955c3ff3d6 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -9426,6 +9426,9 @@  struct qla_qpair *qla2xxx_create_qpair(struct 
scsi_qla_host *vha, int qos,
          qpair->rsp->req = qpair->req;
          qpair->rsp->qpair = qpair;

+        if (!qpair->cpu_mapped)
+            qla_cpu_update(qpair, raw_smp_processor_id());
+
          if (IS_T10_PI_CAPABLE(ha) && ql2xenabledif) {
              if (ha->fw_attributes & BIT_4)
                  qpair->difdix_supported = 1;
diff --git a/drivers/scsi/qla2xxx/qla_inline.h 
b/drivers/scsi/qla2xxx/qla_inline.h
index cce6e425c121..a6e2c7d4ff87 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -538,12 +538,18 @@  qla_mapq_init_qp_cpu_map(struct qla_hw_data *ha,

      if (!ha->qp_cpu_map)
          return;
+
      mask = pci_irq_get_affinity(ha->pdev, msix->vector_base0);
+    if (!mask)
+        return;
+
      qpair->cpuid = cpumask_first(mask);
      for_each_cpu(cpu, mask) {
          ha->qp_cpu_map[cpu] = qpair;
      }
      msix->cpuid = qpair->cpuid;
+
+    qpair->cpu_mapped = true;
  }

  static inline void
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 71feda2cdb63..245e3a5d81fd 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -3770,6 +3770,9 @@  void qla24xx_process_response_queue(struct 
scsi_qla_host *vha,

      if (rsp->qpair->cpuid != smp_processor_id() || 
!rsp->qpair->rcv_intr) {
          rsp->qpair->rcv_intr = 1;
+
+        if (!rsp->qpair->cpu_mapped)
+            qla_cpu_update(rsp->qpair, raw_smp_processor_id());
      }