linux-kernel - Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Oct 2016 09:43:14 +0200
From:   Johannes Thumshirn <jthumshirn@...e.de>
To:     Steffen Maier <maier@...ux.vnet.ibm.com>
Cc:     "Martin K . Petersen" <martin.petersen@...cle.com>,
        Christoph Hellwig <hch@...radead.org>,
        Hannes Reinecke <hare@...e.de>,
        Linux Kernel Mailinglist <linux-kernel@...r.kernel.org>,
        Linux SCSI Mailinglist <linux-scsi@...r.kernel.org>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Anil Gurumurthy <anil.gurumurthy@...gic.com>,
        Sudarsana Kalluru <sudarsana.kalluru@...gic.com>,
        "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Johannes Thumshirn <jth@...nel.org>,
        James Smart <james.smart@...gotech.com>,
        Dick Kennedy <dick.kennedy@...gotech.com>,
        "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" 
        <qla2xxx-upstream@...gic.com>,
        "open list:S390 ZFCP DRIVER" <linux-s390@...r.kernel.org>,
        "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" 
        <linuxppc-dev@...ts.ozlabs.org>,
        "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" 
        <fcoe-devel@...n-fcoe.org>
Subject: Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and
 fc_bsg_job::reply directly

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@...e.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