Re: [PATCH 02/25] qla2xxx: Remove redundant fc_host_port_name call

[Madhani, Himanshu]

Hi Bart, 

> On May 19, 2017, at 3:37 PM, Bart Van Assche  
> wrote:
> 
> On Fri, 2017-05-19 at 14:53 -0700, Himanshu Madhani wrote:
>> From: Quinn Tran 
>> 
>> Remove redundant fc_host_port_name calls to prevent
>> early access of scsi_host->shost_data buffer. This
>> prevent null pointer access.
>> 
>> Cc: 
>> Signed-off-by: Quinn Tran 
>> Signed-off-by: Himanshu Madhani 
>> ---
>> drivers/scsi/qla2xxx/qla_mbx.c | 9 -
>> 1 file changed, 9 deletions(-)
>> 
>> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
>> index a113ab3592a7..12fea77e31c6 100644
>> --- a/drivers/scsi/qla2xxx/qla_mbx.c
>> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
>> @@ -3676,15 +3676,6 @@ qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
>>  qlt_update_host_map(vha, id);
>>  }
>> 
>> -fc_host_port_name(vha->host) =
>> -wwn_to_u64(vha->port_name);
>> -
>> -if (qla_ini_mode_enabled(vha))
>> -ql_dbg(ql_dbg_mbx, vha, 0x1018,
>> -"FA-WWN portname %016llx (%x)\n",
>> -fc_host_port_name(vha->host),
>> -rptid_entry->vp_status);
>> -
>>  set_bit(REGISTER_FC4_NEEDED, >dpc_flags);
>>  set_bit(REGISTER_FDMI_NEEDED, >dpc_flags);
>>  } else {
> 
> Hello Himanshu,
> 
> If the above host port name assignment is redundant, what is the host
> port name assignment it is redundant with?
> 

> Which of the removed statements could trigger a NULL pointer access, and
> from what context? Is there perhaps a call trace available of the NULL
> pointer access?
> 
> Thanks,
> 
> Bart.

We see following NULL pointer access on a faster systems. So far its has been 
only seen on
couple systems in our env. 

[  498.637517] BUG: unable to handle kernel NULL pointer dereference at 
0008
[  498.637572] IP: qla24xx_report_id_acquisition+0x22d/0x3a0 [qla2xxx]

GDB showed following lines were causing NULL pointer deference. 

(gdb) l *qla24xx_report_id_acquisition+0x22d
0x2012d is in qla24xx_report_id_acquisition 
(drivers/scsi/qla2xxx/qla_mbx.c:3679).
3674   }
3675   
3676   
qlt_update_host_map(vha, id);
3677   }
3678   
3679   
fc_host_port_name(vha->host) =
3680   
wwn_to_u64(vha->port_name);
3681   
3682   if 
(qla_ini_mode_enabled(vha))
3683   
ql_dbg(ql_dbg_mbx, vha, 0x1018,
(gdb) 

Thanks,
- Himanshu

Re: [PATCH 02/25] qla2xxx: Remove redundant fc_host_port_name call

[Bart Van Assche]

On Fri, 2017-05-19 at 14:53 -0700, Himanshu Madhani wrote:
> From: Quinn Tran 
> 
> Remove redundant fc_host_port_name calls to prevent
> early access of scsi_host->shost_data buffer. This
> prevent null pointer access.
> 
> Cc: 
> Signed-off-by: Quinn Tran 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/qla_mbx.c | 9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index a113ab3592a7..12fea77e31c6 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -3676,15 +3676,6 @@ qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
>   qlt_update_host_map(vha, id);
>   }
>  
> - fc_host_port_name(vha->host) =
> - wwn_to_u64(vha->port_name);
> -
> - if (qla_ini_mode_enabled(vha))
> - ql_dbg(ql_dbg_mbx, vha, 0x1018,
> - "FA-WWN portname %016llx (%x)\n",
> - fc_host_port_name(vha->host),
> - rptid_entry->vp_status);
> -
>   set_bit(REGISTER_FC4_NEEDED, >dpc_flags);
>   set_bit(REGISTER_FDMI_NEEDED, >dpc_flags);
>   } else {

Hello Himanshu,

If the above host port name assignment is redundant, what is the host
port name assignment it is redundant with?

Which of the removed statements could trigger a NULL pointer access, and
from what context? Is there perhaps a call trace available of the NULL
pointer access?

Thanks,

Bart.

[PATCH 02/25] qla2xxx: Remove redundant fc_host_port_name call

[Himanshu Madhani]

From: Quinn Tran 

Remove redundant fc_host_port_name calls to prevent
early access of scsi_host->shost_data buffer. This
prevent null pointer access.

Cc: 
Signed-off-by: Quinn Tran 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_mbx.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index a113ab3592a7..12fea77e31c6 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -3676,15 +3676,6 @@ qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
qlt_update_host_map(vha, id);
}
 
-   fc_host_port_name(vha->host) =
-   wwn_to_u64(vha->port_name);
-
-   if (qla_ini_mode_enabled(vha))
-   ql_dbg(ql_dbg_mbx, vha, 0x1018,
-   "FA-WWN portname %016llx (%x)\n",
-   fc_host_port_name(vha->host),
-   rptid_entry->vp_status);
-
set_bit(REGISTER_FC4_NEEDED, >dpc_flags);
set_bit(REGISTER_FDMI_NEEDED, >dpc_flags);
} else {
-- 
2.12.0