[1/8] IB/srp: Avoid that duplicate responses trigger a kernel bug

Commit Message

Leon Romanovsky Feb. 13, 2017, 5:54 a.m. UTC

On Sun, Feb 12, 2017 at 08:07:13PM +0000, Bart Van Assche wrote:
> On Sun, 2017-02-12 at 19:05 +0200, Leon Romanovsky wrote:
> > On Fri, Feb 10, 2017 at 03:56:04PM -0800, Bart Van Assche wrote:
> > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> > > index 79bf48477ddb..4068d34f5427 100644
> > > --- a/drivers/infiniband/ulp/srp/ib_srp.c
> > > +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> > > @@ -1899,7 +1899,14 @@ static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp)
> > >  		scmnd = scsi_host_find_tag(target->scsi_host, rsp->tag);
> > >  		if (scmnd) {
> > >  			req = (void *)scmnd->host_scribble;
> > > -			scmnd = srp_claim_req(ch, req, NULL, scmnd);
> > > +			if (req) {
> > > +				scmnd = srp_claim_req(ch, req, NULL, scmnd);
> > > +			} else {
> > > +				shost_printk(KERN_ERR, target->scsi_host,
> > > +					     "NULL host_scribble for response with tag %#llx\n",
> > > +					     rsp->tag);
> > > +				scmnd = NULL;
> > > +			}
> > >  		}
> > >  		if (!scmnd) {
> > >  			shost_printk(KERN_ERR, target->scsi_host,
> >
> > You have the chance to print the message below together with your new
> > print, because scmd will be NULL.
> >
> > What about to do the following check "if (scmd && scmd->host_scribble)"
> > instead of your proposed patch?
>
> That approach would still trigger a kernel oops if a duplicate response is
> received because the second argument of srp_claim_req() must not be NULL.

I'm sure that I'm missing something, but how would it be triggered?
We will enter to call second srp_claim_req() function only if "req" is
not NULL.


>
> Bart.
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
>
> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Comments

Bart Van Assche Feb. 13, 2017, 4:02 p.m. UTC | #1

On Mon, 2017-02-13 at 07:54 +0200, Leon Romanovsky wrote:
> I'm sure that I'm missing something, but how would it be triggered?
> We will enter to call second srp_claim_req() function only if "req" is
> not NULL.
> 
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 79bf48477ddb..40e7f27c40bf 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -1897,10 +1897,12 @@ static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp)
>  		complete(&ch->tsk_mgmt_done);
>  	} else {
>  		scmnd = scsi_host_find_tag(target->scsi_host, rsp->tag);
> -		if (scmnd) {
> +		if (scmnd && scmnd->host_scribble) {
>  			req = (void *)scmnd->host_scribble;
>  			scmnd = srp_claim_req(ch, req, NULL, scmnd);
>  		}
> +		else
> +			scnmnd = NULL;
>  		if (!scmnd) {
>  			shost_printk(KERN_ERR, target->scsi_host,
>  				     "Null scmnd for RSP w/tag %#016llx received on ch %td / QP %#x\n",

Hello Leon,

Sorry but I had misread your previous e-mail. I agree that the above should
work fine.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 79bf48477ddb..40e7f27c40bf 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1897,10 +1897,12 @@  static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp)
 		complete(&ch->tsk_mgmt_done);
 	} else {
 		scmnd = scsi_host_find_tag(target->scsi_host, rsp->tag);
-		if (scmnd) {
+		if (scmnd && scmnd->host_scribble) {
 			req = (void *)scmnd->host_scribble;
 			scmnd = srp_claim_req(ch, req, NULL, scmnd);
 		}
+		else
+			scnmnd = NULL;
 		if (!scmnd) {
 			shost_printk(KERN_ERR, target->scsi_host,
 				     "Null scmnd for RSP w/tag %#016llx received on ch %td / QP %#x\n",