From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "jthumshirn@suse.de" <jthumshirn@suse.de>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>
CC: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "hch@lst.de" <hch@lst.de>, "hare@suse.de" <hare@suse.de>,
        Bart Van Assche <Bart.VanAssche@wdc.com>
Subject: Re: [PATCH] scsi: fc: check for rport presence in fc_block_scsi_eh
Thread-Topic: [PATCH] scsi: fc: check for rport presence in fc_block_scsi_eh
Thread-Index: AQHTNfX3dx9Qm2R5+UWSH2J8pKB4u6LGQDwA
Date: Mon, 25 Sep 2017 23:29:07 +0000
Message-ID: <1506382146.2641.61.camel@wdc.com>
References: <20170925120049.19689-1-jthumshirn@suse.de>
In-Reply-To: <20170925120049.19689-1-jthumshirn@suse.de>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-ID: <55FAF15C62D8C14C81FDE6D45A3AEB8B@namprd04.prod.outlook.com>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Sep 2017 23:29:08.0005
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0401MB1536
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by nfs id v8PNTgjc016584
Content-Length: 1150
Lines: 32

On Mon, 2017-09-25 at 14:00 +0200, Johannes Thumshirn wrote:
> Coverity-scan recently found a possible NULL pointer dereference in
> fc_block_scsi_eh() as starget_to_rport() either returns the rport for
> the startget or NULL.
> 
> While it is rather unlikely to have fc_block_scsi_eh() called without
> an rport associated it's a good idea to catch potential misuses of the
> API gracefully.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  drivers/scsi/scsi_transport_fc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index ba9d70f8a6a1..830ce53f30fb 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -3328,6 +3328,9 @@ int fc_block_scsi_eh(struct scsi_cmnd *cmnd)
>  {
>  	struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device));
>  
> +	if (WARN_ON(!rport))
> +		return 0;
> +
>  	return fc_block_rport(rport);
>  }
>  EXPORT_SYMBOL(fc_block_scsi_eh);

Did you perhaps intend to use WARN_ON_ONCE() instead of WARN_ON()? Anyway:

Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>