Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly

[Johannes Thumshirn]

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@xxxxxxx                                +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