diff mbox series

qedf: Added NULL pointer checks in qedf_update_link_speed().

Message ID 20210512072533.23618-1-jhasan@marvell.com
State New
Headers show
Series qedf: Added NULL pointer checks in qedf_update_link_speed(). | expand

Commit Message

Javed Hasan May 12, 2021, 7:25 a.m. UTC 
Issue :- BUG: unable to handle kernel NULL pointer dereference at 000000000000003c
 On installation of RHEL-8.3.0-20200820.n.0 distro below stack
 was generating on error.

 [   14.042059] Call Trace:
 [   14.042061]  <IRQ>
 [   14.042068]  qedf_link_update+0x144/0x1f0 [qedf]
 [   14.042117]  qed_link_update+0x5c/0x80 [qed]
 [   14.042135]  qed_mcp_handle_link_change+0x2d2/0x410 [qed]
 [   14.042155]  ? qed_set_ptt+0x70/0x80 [qed]
 [   14.042170]  ? qed_set_ptt+0x70/0x80 [qed]
 [   14.042186]  ? qed_rd+0x13/0x40 [qed]
 [   14.042205]  qed_mcp_handle_events+0x437/0x690 [qed]
 [   14.042221]  ? qed_set_ptt+0x70/0x80 [qed]
 [   14.042239]  qed_int_sp_dpc+0x3a6/0x3e0 [qed]
 [   14.042245]  tasklet_action_common.isra.14+0x5a/0x100
 [   14.042250]  __do_softirq+0xe4/0x2f8
 [   14.042253]  irq_exit+0xf7/0x100
 [   14.042255]  do_IRQ+0x7f/0xd0
 [   14.042257]  common_interrupt+0xf/0xf
 [   14.042259]  </IRQ>

 Root cause :- API qedf_link_update() is getting called from QED.
  but by that time shost_data is not initialised. That is leading NULL pointer dereference
  when we try to derefference shost_data while updating supported_speeds.

  fc_host_supported_speeds(lport->host) = lport->link_supported_speeds;

 Expansion of fc_host_supported_speeds.
 #define fc_host_supported_speeds(x)	\
  (((struct fc_host_attrs *)(x)->shost_data)->supported_speeds)

 Fix :- Added NULL pointer check for shost_data.

Signed-off-by: Javed Hasan <jhasan@marvell.com>

Comments

Himanshu Madhani May 12, 2021, 9:31 p.m. UTC | #1 
> On May 12, 2021, at 2:25 AM, Javed Hasan <jhasan@marvell.com> wrote:

> 

> Issue :- BUG: unable to handle kernel NULL pointer dereference at 000000000000003c

> On installation of RHEL-8.3.0-20200820.n.0 distro below stack

> was generating on error.

> 

> [   14.042059] Call Trace:

> [   14.042061]  <IRQ>

> [   14.042068]  qedf_link_update+0x144/0x1f0 [qedf]

> [   14.042117]  qed_link_update+0x5c/0x80 [qed]

> [   14.042135]  qed_mcp_handle_link_change+0x2d2/0x410 [qed]

> [   14.042155]  ? qed_set_ptt+0x70/0x80 [qed]

> [   14.042170]  ? qed_set_ptt+0x70/0x80 [qed]

> [   14.042186]  ? qed_rd+0x13/0x40 [qed]

> [   14.042205]  qed_mcp_handle_events+0x437/0x690 [qed]

> [   14.042221]  ? qed_set_ptt+0x70/0x80 [qed]

> [   14.042239]  qed_int_sp_dpc+0x3a6/0x3e0 [qed]

> [   14.042245]  tasklet_action_common.isra.14+0x5a/0x100

> [   14.042250]  __do_softirq+0xe4/0x2f8

> [   14.042253]  irq_exit+0xf7/0x100

> [   14.042255]  do_IRQ+0x7f/0xd0

> [   14.042257]  common_interrupt+0xf/0xf

> [   14.042259]  </IRQ>

> 

> Root cause :- API qedf_link_update() is getting called from QED.

>  but by that time shost_data is not initialised. That is leading NULL pointer dereference

>  when we try to derefference shost_data while updating supported_speeds.

> 

>  fc_host_supported_speeds(lport->host) = lport->link_supported_speeds;

> 

> Expansion of fc_host_supported_speeds.

> #define fc_host_supported_speeds(x)	\

>  (((struct fc_host_attrs *)(x)->shost_data)->supported_speeds)

> 

> Fix :- Added NULL pointer check for shost_data.

> 

> Signed-off-by: Javed Hasan <jhasan@marvell.com>

> 

> diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c

> index 69f7784233f9..756231151882 100644

> --- a/drivers/scsi/qedf/qedf_main.c

> +++ b/drivers/scsi/qedf/qedf_main.c

> @@ -536,7 +536,9 @@ static void qedf_update_link_speed(struct qedf_ctx *qedf,

> 	if (linkmode_intersects(link->supported_caps, sup_caps))

> 		lport->link_supported_speeds |= FC_PORTSPEED_20GBIT;

> 

> -	fc_host_supported_speeds(lport->host) = lport->link_supported_speeds;

> +	if (lport->host && lport->host->shost_data)

> +		fc_host_supported_speeds(lport->host) =

> +			lport->link_supported_speeds;

> }

> 

> static void qedf_bw_update(void *dev)

> -- 

> 2.18.2

> 


This should be sent to stable with 

Fixes: 61d8658b4a435 ("scsi: qedf: Add QLogic FastLinQ offload FCoE driver framework.”)
Cc: <stable@vger.kernel.org>

With above added, Looks Good. 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>


--
Himanshu Madhani	 Oracle Linux Engineering
Martin K. Petersen May 15, 2021, 3:13 a.m. UTC | #2 
On Wed, 12 May 2021 00:25:33 -0700, Javed Hasan wrote:

>  Issue :- BUG: unable to handle kernel NULL pointer dereference at 000000000000003c

>  On installation of RHEL-8.3.0-20200820.n.0 distro below stack

>  was generating on error.

> 

>  [   14.042059] Call Trace:

>  [   14.042061]  <IRQ>

>  [   14.042068]  qedf_link_update+0x144/0x1f0 [qedf]

>  [   14.042117]  qed_link_update+0x5c/0x80 [qed]

>  [   14.042135]  qed_mcp_handle_link_change+0x2d2/0x410 [qed]

>  [   14.042155]  ? qed_set_ptt+0x70/0x80 [qed]

>  [   14.042170]  ? qed_set_ptt+0x70/0x80 [qed]

>  [   14.042186]  ? qed_rd+0x13/0x40 [qed]

>  [   14.042205]  qed_mcp_handle_events+0x437/0x690 [qed]

>  [   14.042221]  ? qed_set_ptt+0x70/0x80 [qed]

>  [   14.042239]  qed_int_sp_dpc+0x3a6/0x3e0 [qed]

>  [   14.042245]  tasklet_action_common.isra.14+0x5a/0x100

>  [   14.042250]  __do_softirq+0xe4/0x2f8

>  [   14.042253]  irq_exit+0xf7/0x100

>  [   14.042255]  do_IRQ+0x7f/0xd0

>  [   14.042257]  common_interrupt+0xf/0xf

>  [   14.042259]  </IRQ>

> 

> [...]


Applied to 5.13/scsi-fixes, thanks!

[1/1] qedf: Added NULL pointer checks in qedf_update_link_speed().
      https://git.kernel.org/mkp/scsi/c/73578af92a0f

-- 
Martin K. Petersen	Oracle Linux Engineering
Martin K. Petersen May 15, 2021, 10:14 p.m. UTC | #3 
On Wed, 12 May 2021 00:25:33 -0700, Javed Hasan wrote:

>  Issue :- BUG: unable to handle kernel NULL pointer dereference at 000000000000003c

>  On installation of RHEL-8.3.0-20200820.n.0 distro below stack

>  was generating on error.

> 

>  [   14.042059] Call Trace:

>  [   14.042061]  <IRQ>

>  [   14.042068]  qedf_link_update+0x144/0x1f0 [qedf]

>  [   14.042117]  qed_link_update+0x5c/0x80 [qed]

>  [   14.042135]  qed_mcp_handle_link_change+0x2d2/0x410 [qed]

>  [   14.042155]  ? qed_set_ptt+0x70/0x80 [qed]

>  [   14.042170]  ? qed_set_ptt+0x70/0x80 [qed]

>  [   14.042186]  ? qed_rd+0x13/0x40 [qed]

>  [   14.042205]  qed_mcp_handle_events+0x437/0x690 [qed]

>  [   14.042221]  ? qed_set_ptt+0x70/0x80 [qed]

>  [   14.042239]  qed_int_sp_dpc+0x3a6/0x3e0 [qed]

>  [   14.042245]  tasklet_action_common.isra.14+0x5a/0x100

>  [   14.042250]  __do_softirq+0xe4/0x2f8

>  [   14.042253]  irq_exit+0xf7/0x100

>  [   14.042255]  do_IRQ+0x7f/0xd0

>  [   14.042257]  common_interrupt+0xf/0xf

>  [   14.042259]  </IRQ>

> 

> [...]


Applied to 5.13/scsi-fixes, thanks!

[1/1] qedf: Added NULL pointer checks in qedf_update_link_speed().
      https://git.kernel.org/mkp/scsi/c/73578af92a0f

-- 
Martin K. Petersen	Oracle Linux Engineering
diff mbox series

Patch

diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 69f7784233f9..756231151882 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -536,7 +536,9 @@  static void qedf_update_link_speed(struct qedf_ctx *qedf,
 	if (linkmode_intersects(link->supported_caps, sup_caps))
 		lport->link_supported_speeds |= FC_PORTSPEED_20GBIT;
 
-	fc_host_supported_speeds(lport->host) = lport->link_supported_speeds;
+	if (lport->host && lport->host->shost_data)
+		fc_host_supported_speeds(lport->host) =
+			lport->link_supported_speeds;
 }
 
 static void qedf_bw_update(void *dev)