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

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2986 bytes --]

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.

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",

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]