Just for the records: There's another bug below.

On 07/25/2017 04:14 PM, Steffen Maier wrote:
The scsi_device argument of zfcp_fc_fcp_tm() can now be NULL.

In zfcp_fsf_fcp_task_mgmt() resolve the still old argument scsi_cmnd
into scsi_device very early and only depend on scsi_device and derived
objects in the function body.

Scsi_device and derived zfcp_scsi_dev can later be NULL for the
target reset case, so do not depend on them unconditionally.
For the generic case, rather change to using zfcp_port directly.

This prepares to later change the function signature replacing the
scsi_cmnd argument with zfcp_port and an
optional scsi_device which can be NULL.

Signed-off-by: Steffen Maier <ma...@linux.vnet.ibm.com>
---
  drivers/s390/scsi/zfcp_fc.h  |  6 ++++--
  drivers/s390/scsi/zfcp_fsf.c | 25 +++++++++++++++++--------
  2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index f221a34c26df..2dc7d2a6f6ea 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2339,13 +2339,19 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct 
scsi_cmnd *scmnd,
  {
        struct zfcp_fsf_req *req = NULL;
        struct fcp_cmnd *fcp_cmnd;
-       struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scmnd->device);
-       struct zfcp_qdio *qdio = zfcp_sdev->port->adapter->qdio;
+       struct scsi_device *sdev = scmnd->device;
+       struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);

BUG: must not unconditionally dereference sdev which can be NULL later on in the patch set!

Fix: +  struct zfcp_scsi_dev *zfcp_sdev = sdev ? sdev_to_zfcp(sdev) : NULL;

Fix is no longer necessary in my reworked v2 (always having a non-NULL sdev) to be sent when I successfully completed function test.

+       struct zfcp_port *port = zfcp_sdev->port;

This line was removed in the subsequent patch 5/9, so here the unconditional deref is OK because here in this patch we still get a non-NULL sdev. (The line is just argument lifting preparing for the function argument replacement in 5/9.)

Other accesses to sdev or zfcp_sdev were properly guarded with this patch.

--
Mit freundlichen Grüßen / Kind regards
Steffen Maier

Linux on z Systems Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Reply via email to