Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755250AbcJYHnY (ORCPT ); Tue, 25 Oct 2016 03:43:24 -0400 Received: from mx2.suse.de ([195.135.220.15]:57560 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750823AbcJYHnW (ORCPT ); Tue, 25 Oct 2016 03:43:22 -0400 Date: Tue, 25 Oct 2016 09:43:14 +0200 From: Johannes Thumshirn To: Steffen Maier Cc: "Martin K . Petersen" , Christoph Hellwig , Hannes Reinecke , Linux Kernel Mailinglist , Linux SCSI Mailinglist , Martin Schwidefsky , Heiko Carstens , Anil Gurumurthy , Sudarsana Kalluru , "James E.J. Bottomley" , Tyrel Datwyler , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Johannes Thumshirn , James Smart , Dick Kennedy , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" , "open list:S390 ZFCP DRIVER" , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" , "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" Subject: Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly Message-ID: <20161025074313.4ma4czgdj46imaei@linux-x5ow.site> References: <20161014073821.qvkpjtwraqnwzzs3@linux-x5ow.site> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20161014073821.qvkpjtwraqnwzzs3@linux-x5ow.site> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2610 Lines: 78 On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > Hm, still behaves for me like I reported for v2: > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > > Hi Steffen, > > Can you please try the following on top of 2/16? > > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c > index 4149dac..baebaab 100644 > --- a/drivers/scsi/scsi_transport_fc.c > +++ b/drivers/scsi/scsi_transport_fc.c > @@ -3786,6 +3786,12 @@ enum fc_dispatch_result { > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > + /* check if we really have all the request data needed */ > + if (job->request_len < cmdlen) { > + ret = -ENOMSG; > + goto fail_host_msg; > + } > + > /* Validate the host command */ > switch (bsg_request->msgcode) { > case FC_BSG_HST_ADD_RPORT: > @@ -3831,12 +3837,6 @@ enum fc_dispatch_result { > goto fail_host_msg; > } > > - /* check if we really have all the request data needed */ > - if (job->request_len < cmdlen) { > - ret = -ENOMSG; > - goto fail_host_msg; > - } > - > ret = i->f->bsg_request(job); > if (!ret) > return FC_DISPATCH_UNLOCKED; > @@ -3887,6 +3887,12 @@ enum fc_dispatch_result { > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > + /* check if we really have all the request data needed */ > + if (job->request_len < cmdlen) { > + ret = -ENOMSG; > + goto fail_rport_msg; > + } > + > /* Validate the rport command */ > switch (bsg_request->msgcode) { > case FC_BSG_RPT_ELS: > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > job->request as req->cmd and job->request_len = req->cmd_len. But without > checkinf job->request_len we don't know whether we're save to touch > job->request (a.k.a. bsg_request). Hi Steffen, Did you have any chance testing this? I hacked fcping to work with non-FCoE and rports as well and tested with FCoE and lpfc. No problems seen from my side. I've also pused the series (With this change folded in) to my git tree at [1] if this helps you in any way. [1] https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 Thanks a lot, Johannes -- Johannes Thumshirn Storage jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850