Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1213281 > unrolled thread

Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

Started byEwan Milne <emilne@redhat.com>
First post2015-08-25 22:00 +0200
Last post2015-08-31 10:10 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable  check Ewan Milne <emilne@redhat.com> - 2015-08-25 22:00 +0200
    RE: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable  check "Winkler, Tomas" <tomas.winkler@intel.com> - 2015-08-25 23:10 +0200
      Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable  check James Bottomley <James.Bottomley@HansenPartnership.com> - 2015-08-27 01:40 +0200
        Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable  check Ewan Milne <emilne@redhat.com> - 2015-08-27 22:00 +0200
          RE: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable  check "Winkler, Tomas" <tomas.winkler@intel.com> - 2015-08-30 12:40 +0200
            Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable  check James Bottomley <James.Bottomley@HansenPartnership.com> - 2015-08-30 19:00 +0200
              RE: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable  check "Winkler, Tomas" <tomas.winkler@intel.com> - 2015-08-31 10:10 +0200

#1213281 — Re: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check

FromEwan Milne <emilne@redhat.com>
Date2015-08-25 22:00 +0200
SubjectRe: [scsi 5/7 RESEND] scsi_debug: schedule_resp fix input variable check
Message-ID<q1sxX-870-3@gated-at.bofh.it>
On Tue, 2015-07-28 at 16:54 +0300, Tomas Winkler wrote:
> The function should never be called with cmnd NULL so
> put a fat WARN there.
> Fix also smatch wraning:
> schedule_resp() warn: variable dereferenced before check 'cmnd'
> 
> Cc: Douglas Gilbert <dgilbert@interlog.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Acked-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
>  drivers/scsi/scsi_debug.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 3a70683cf9f9..faa4ddd8decf 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -3941,13 +3941,20 @@ schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
>  	unsigned long iflags;
>  	int k, num_in_q, qdepth, inject;
>  	struct sdebug_queued_cmd *sqcp = NULL;
> -	struct scsi_device *sdp = cmnd->device;
> +	struct scsi_device *sdp;
> +
> +	/* this should never happen */
> +	if (WARN_ON(!cmnd))
> +		return SCSI_MLQUEUE_HOST_BUSY;
>  
> -	if (NULL == cmnd || NULL == devip) {
> -		pr_warn("called with NULL cmnd or devip pointer\n");
> +	if (NULL == devip) {
> +		pr_warn("called devip == NULL\n");
>  		/* no particularly good error to report back */
>  		return SCSI_MLQUEUE_HOST_BUSY;
>  	}

Please refer to the patch I just posted, we can't return _HOST_BUSY here
if devip == NULL.  I posted a fix against the current "misc" branch as
I don't see this patch applied, let me know if I need to update it.

> +
> +	sdp = cmnd->device;
> +
>  	if ((scsi_result) && (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts))
>  		sdev_printk(KERN_INFO, sdp, "%s: non-zero result=0x%x\n",
>  			    __func__, scsi_result);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1213355

From"Winkler, Tomas" <tomas.winkler@intel.com>
Date2015-08-25 23:10 +0200
Message-ID<q1tDI-1uW-35@gated-at.bofh.it>
In reply to#1213281
DQo+ID4gKwkvKiB0aGlzIHNob3VsZCBuZXZlciBoYXBwZW4gKi8NCj4gPiArCWlmIChXQVJOX09O
KCFjbW5kKSkNCj4gPiArCQlyZXR1cm4gU0NTSV9NTFFVRVVFX0hPU1RfQlVTWTsNCj4gPg0KPiA+
IC0JaWYgKE5VTEwgPT0gY21uZCB8fCBOVUxMID09IGRldmlwKSB7DQo+ID4gLQkJcHJfd2Fybigi
Y2FsbGVkIHdpdGggTlVMTCBjbW5kIG9yIGRldmlwIHBvaW50ZXJcbiIpOw0KPiA+ICsJaWYgKE5V
TEwgPT0gZGV2aXApIHsNCj4gPiArCQlwcl93YXJuKCJjYWxsZWQgZGV2aXAgPT0gTlVMTFxuIik7
DQo+ID4gIAkJLyogbm8gcGFydGljdWxhcmx5IGdvb2QgZXJyb3IgdG8gcmVwb3J0IGJhY2sgKi8N
Cj4gPiAgCQlyZXR1cm4gU0NTSV9NTFFVRVVFX0hPU1RfQlVTWTsNCj4gPiAgCX0NCj4gDQo+IFBs
ZWFzZSByZWZlciB0byB0aGUgcGF0Y2ggSSBqdXN0IHBvc3RlZCwgd2UgY2FuJ3QgcmV0dXJuIF9I
T1NUX0JVU1kgaGVyZQ0KPiBpZiBkZXZpcCA9PSBOVUxMLiAgSSBwb3N0ZWQgYSBmaXggYWdhaW5z
dCB0aGUgY3VycmVudCAibWlzYyIgYnJhbmNoIGFzDQo+IEkgZG9uJ3Qgc2VlIHRoaXMgcGF0Y2gg
YXBwbGllZCwgbGV0IG1lIGtub3cgaWYgSSBuZWVkIHRvIHVwZGF0ZSBpdC4NCg0KDQpJJ20ganVz
dCBub3Qgc3VyZSB3aHkgdGhlIHBhdGNoZXMgYXJlIG5vdCBtZXJnZWQgb3IgZXZlbiByZWplY3Rl
ZC4NCkknbSBzdWJtaXR0aW5nIHBhdGNoZXMgdG8gdGhlIExpbnV4IGtlcm5lbCBmb3IgbW9yZSB0
aGFuIDEwIHllYXJzIHRvIHZhcmlvdXMgdHJlZXMgYW5kICBJIGNhbiBhZ3JlZSB0aGF0IHRoZXNl
IGFyZSBub3Qgc29tZSAgdXJnZW50IGZpeGVzLCBidXQgIHRoaXMgaXMgdGhlIGZpcnN0IHRpbWUg
bXkgZWZmb3J0IGlzIGlnbm9yZWQgZm9yIHN1Y2ggbG9uZyB0aW1lIGJ5IHRoZSBtYWludGFpbmVy
LiANCg0KVGhhbmtzDQpUb21hcw0KDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214243

FromJames Bottomley <James.Bottomley@HansenPartnership.com>
Date2015-08-27 01:40 +0200
Message-ID<q1Ssp-3Ko-11@gated-at.bofh.it>
In reply to#1213355
On Tue, 2015-08-25 at 21:03 +0000, Winkler, Tomas wrote:
> 
> > > +	/* this should never happen */
> > > +	if (WARN_ON(!cmnd))
> > > +		return SCSI_MLQUEUE_HOST_BUSY;
> > >
> > > -	if (NULL == cmnd || NULL == devip) {
> > > -		pr_warn("called with NULL cmnd or devip pointer\n");
> > > +	if (NULL == devip) {
> > > +		pr_warn("called devip == NULL\n");
> > >  		/* no particularly good error to report back */
> > >  		return SCSI_MLQUEUE_HOST_BUSY;
> > >  	}
> > 
> > Please refer to the patch I just posted, we can't return _HOST_BUSY here
> > if devip == NULL.  I posted a fix against the current "misc" branch as
> > I don't see this patch applied, let me know if I need to update it.
> 
> 
> I'm just not sure why the patches are not merged or even rejected.

Because ideally I want a Maintainer ack.  That's Doug Gilbert.

> I'm submitting patches to the Linux kernel for more than 10 years to
> various trees and  I can agree that these are not some  urgent fixes,
> but  this is the first time my effort is ignored for such long time by
> the maintainer. 

Well, OK, I trust martin, I'll override the lack of Maintainer ack if
you fix as Ewan suggests.

James


> Thanks
> Tomas
> 
> NrybXǧv^)޺{.n+{"{ayʇڙ,jfhzwj:+vwjmzZ+ݢj"!



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214812

FromEwan Milne <emilne@redhat.com>
Date2015-08-27 22:00 +0200
Message-ID<q2bv5-5V4-11@gated-at.bofh.it>
In reply to#1214243
On Wed, 2015-08-26 at 16:38 -0700, James Bottomley wrote:
> On Tue, 2015-08-25 at 21:03 +0000, Winkler, Tomas wrote:
> > 
> > > > +	/* this should never happen */
> > > > +	if (WARN_ON(!cmnd))
> > > > +		return SCSI_MLQUEUE_HOST_BUSY;
> > > >
> > > > -	if (NULL == cmnd || NULL == devip) {
> > > > -		pr_warn("called with NULL cmnd or devip pointer\n");
> > > > +	if (NULL == devip) {
> > > > +		pr_warn("called devip == NULL\n");
> > > >  		/* no particularly good error to report back */
> > > >  		return SCSI_MLQUEUE_HOST_BUSY;
> > > >  	}
> > > 
> > > Please refer to the patch I just posted, we can't return _HOST_BUSY here
> > > if devip == NULL.  I posted a fix against the current "misc" branch as
> > > I don't see this patch applied, let me know if I need to update it.
> > 
> > 
> > I'm just not sure why the patches are not merged or even rejected.
> 
> Because ideally I want a Maintainer ack.  That's Doug Gilbert.
> 
> > I'm submitting patches to the Linux kernel for more than 10 years to
> > various trees and  I can agree that these are not some  urgent fixes,
> > but  this is the first time my effort is ignored for such long time by
> > the maintainer. 
> 
> Well, OK, I trust martin, I'll override the lack of Maintainer ack if
> you fix as Ewan suggests.
> 

Just to clarify, I didn't have a problem with Tomas' patch per se, it's
just that my patch won't apply on top of his.  I'll submit a v2 if you
want, so you can apply Tomas' patch first.  The problem I'm fixing has
been in there for a while.  Let me know if you want me to do that.

-Ewan

> James
> 
> 
> > Thanks
> > Tomas
> > 
> > NrybXǧv^)޺{.n+{"{ayʇڙ,jfhzwj:+vwjmzZ+ݢj"!
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215906

From"Winkler, Tomas" <tomas.winkler@intel.com>
Date2015-08-30 12:40 +0200
Message-ID<q38bL-6q2-5@gated-at.bofh.it>
In reply to#1214812
PiA+ID4NCj4gPiA+DQo+ID4gPiBJJ20ganVzdCBub3Qgc3VyZSB3aHkgdGhlIHBhdGNoZXMgYXJl
IG5vdCBtZXJnZWQgb3IgZXZlbiByZWplY3RlZC4NCj4gPg0KPiA+IEJlY2F1c2UgaWRlYWxseSBJ
IHdhbnQgYSBNYWludGFpbmVyIGFjay4gIFRoYXQncyBEb3VnIEdpbGJlcnQuDQoNCkphbWVzIA0K
VGhlIHBhdGNoZXMgd2VyZSBkaXNjdXNzZWQgYW5kIHRoZSBBQ2tlZCBieSBEb3VnIGluIEZlYnJ1
YXJ5IGFuZCByZXZpZXdlZCBhZ2FpbiBhZnRlciByZXNlbmQgIGJ5IE1hcnRpbiBQZXRlcnNlbiAN
Cg0KPiA+DQo+ID4gPiBJJ20gc3VibWl0dGluZyBwYXRjaGVzIHRvIHRoZSBMaW51eCBrZXJuZWwg
Zm9yIG1vcmUgdGhhbiAxMCB5ZWFycyB0bw0KPiA+ID4gdmFyaW91cyB0cmVlcyBhbmQgIEkgY2Fu
IGFncmVlIHRoYXQgdGhlc2UgYXJlIG5vdCBzb21lICB1cmdlbnQgZml4ZXMsDQo+ID4gPiBidXQg
IHRoaXMgaXMgdGhlIGZpcnN0IHRpbWUgbXkgZWZmb3J0IGlzIGlnbm9yZWQgZm9yIHN1Y2ggbG9u
ZyB0aW1lIGJ5DQo+ID4gPiB0aGUgbWFpbnRhaW5lci4NCj4gPg0KPiA+IFdlbGwsIE9LLCBJIHRy
dXN0IG1hcnRpbiwgSSdsbCBvdmVycmlkZSB0aGUgbGFjayBvZiBNYWludGFpbmVyIGFjayBpZg0K
PiA+IHlvdSBmaXggYXMgRXdhbiBzdWdnZXN0cy4NCg0KPiANCj4gSnVzdCB0byBjbGFyaWZ5LCBJ
IGRpZG4ndCBoYXZlIGEgcHJvYmxlbSB3aXRoIFRvbWFzJyBwYXRjaCBwZXIgc2UsIGl0J3MNCj4g
anVzdCB0aGF0IG15IHBhdGNoIHdvbid0IGFwcGx5IG9uIHRvcCBvZiBoaXMuICBJJ2xsIHN1Ym1p
dCBhIHYyIGlmIHlvdQ0KPiB3YW50LCBzbyB5b3UgY2FuIGFwcGx5IFRvbWFzJyBwYXRjaCBmaXJz
dC4gIFRoZSBwcm9ibGVtIEknbSBmaXhpbmcgaGFzDQo+IGJlZW4gaW4gdGhlcmUgZm9yIGEgd2hp
bGUuICBMZXQgbWUga25vdyBpZiB5b3Ugd2FudCBtZSB0byBkbyB0aGF0Lg0KPiANCkhvcGVmdWxs
eSB5b3VyIHBhdGNoIHNob3VsZCBiZSBhcHBsaWVkIGFib3ZlIG15IHBhdGNoZXMgYnV0IEkgY2Fu
IG1ha2UgdGhhdCBlZmZvcnQgYW5kIHN0YWNrIHRoZW0gb24gdG9wIG9mIHlvdXJzLg0KVGhhbmtz
DQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215945

FromJames Bottomley <James.Bottomley@HansenPartnership.com>
Date2015-08-30 19:00 +0200
Message-ID<q3e7v-6ov-1@gated-at.bofh.it>
In reply to#1215906
On Sun, 2015-08-30 at 10:36 +0000, Winkler, Tomas wrote:
> > > >
> > > >
> > > > I'm just not sure why the patches are not merged or even rejected.
> > >
> > > Because ideally I want a Maintainer ack.  That's Doug Gilbert.
> 
> James 
> The patches were discussed and the ACked by Doug in February

7/7 wasn't.

>  and reviewed again after resend  by Martin Petersen 

Yes, that's why I'm willing to override the lack of maintainer ack.
I'll give Doug until Monday first.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216082

From"Winkler, Tomas" <tomas.winkler@intel.com>
Date2015-08-31 10:10 +0200
Message-ID<q3ska-1RI-19@gated-at.bofh.it>
In reply to#1215945
PiANCj4gT24gU3VuLCAyMDE1LTA4LTMwIGF0IDEwOjM2ICswMDAwLCBXaW5rbGVyLCBUb21hcyB3
cm90ZToNCj4gPiA+ID4gPg0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gSSdtIGp1c3Qgbm90IHN1cmUg
d2h5IHRoZSBwYXRjaGVzIGFyZSBub3QgbWVyZ2VkIG9yIGV2ZW4gcmVqZWN0ZWQuDQo+ID4gPiA+
DQo+ID4gPiA+IEJlY2F1c2UgaWRlYWxseSBJIHdhbnQgYSBNYWludGFpbmVyIGFjay4gIFRoYXQn
cyBEb3VnIEdpbGJlcnQuDQo+ID4NCj4gPiBKYW1lcw0KPiA+IFRoZSBwYXRjaGVzIHdlcmUgZGlz
Y3Vzc2VkIGFuZCB0aGUgQUNrZWQgYnkgRG91ZyBpbiBGZWJydWFyeQ0KPiANCj4gNy83IHdhc24n
dC4NCkphbWVzLCBmYWlyIGVub3VnaCwgIHRoZSByZXNldCB0aG91Z2ggY2FuIGJlIG1lcmdlZCwg
cmlnaHQgPyANCg0KPiANCj4gPiAgYW5kIHJldmlld2VkIGFnYWluIGFmdGVyIHJlc2VuZCAgYnkg
TWFydGluIFBldGVyc2VuDQo+IA0KPiBZZXMsIHRoYXQncyB3aHkgSSdtIHdpbGxpbmcgdG8gb3Zl
cnJpZGUgdGhlIGxhY2sgb2YgbWFpbnRhaW5lciBhY2suDQo+IEknbGwgZ2l2ZSBEb3VnIHVudGls
IE1vbmRheSBmaXJzdC4NCg0KT2theS4NCg0KVGhhbmtzDQpUb21hcw0KDQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web