| Message ID | 20190118212931.18482-1-skhan@linuxfoundation.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usbip: Fix vep_free_request() null pointer checks on input args | expand |
On Fri, Jan 18, 2019 at 02:29:30PM -0700, Shuah Khan wrote: > From: Shuah Khan <shuah@kernel.org> > > Fix vep_free_request() to return when usb_ep and usb_request are null > instead of calling WARN_ON. > > Signed-off-by: Shuah Khan <shuah@kernel.org> > --- > drivers/usb/usbip/vudc_dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c > index 1634d8698e15..bfc8218e3fb6 100644 > --- a/drivers/usb/usbip/vudc_dev.c > +++ b/drivers/usb/usbip/vudc_dev.c > @@ -297,7 +297,7 @@ static void vep_free_request(struct usb_ep *_ep, struct usb_request *_req) > { > struct vrequest *req; > > - if (WARN_ON(!_ep || !_req)) > + if (!_ep || !_req) It's impossible for _ep to be NULL in this callback (see usb_ep_free_request() for where this is called from to prove that), so I don't think you need to check that. It's almost impossible for _req to be NULL, so you might as well leave that check in. thanks, greg k-h
On 1/19/19 1:17 AM, Greg KH wrote: > On Fri, Jan 18, 2019 at 02:29:30PM -0700, Shuah Khan wrote: >> From: Shuah Khan <shuah@kernel.org> >> >> Fix vep_free_request() to return when usb_ep and usb_request are null >> instead of calling WARN_ON. >> >> Signed-off-by: Shuah Khan <shuah@kernel.org> >> --- >> drivers/usb/usbip/vudc_dev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c >> index 1634d8698e15..bfc8218e3fb6 100644 >> --- a/drivers/usb/usbip/vudc_dev.c >> +++ b/drivers/usb/usbip/vudc_dev.c >> @@ -297,7 +297,7 @@ static void vep_free_request(struct usb_ep *_ep, struct usb_request *_req) >> { >> struct vrequest *req; >> >> - if (WARN_ON(!_ep || !_req)) >> + if (!_ep || !_req) > > It's impossible for _ep to be NULL in this callback (see > usb_ep_free_request() for where this is called from to prove that), so I > don't think you need to check that. It's almost impossible for _req to > be NULL, so you might as well leave that check in. > Yes. ep can never be null here in vep_free_request(). I will leave this alone. thanks, -- Shuah
On Tue, Jan 22, 2019 at 04:05:28PM -0700, shuah wrote: > On 1/19/19 1:17 AM, Greg KH wrote: > > On Fri, Jan 18, 2019 at 02:29:30PM -0700, Shuah Khan wrote: > > > From: Shuah Khan <shuah@kernel.org> > > > > > > Fix vep_free_request() to return when usb_ep and usb_request are null > > > instead of calling WARN_ON. > > > > > > Signed-off-by: Shuah Khan <shuah@kernel.org> > > > --- > > > drivers/usb/usbip/vudc_dev.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c > > > index 1634d8698e15..bfc8218e3fb6 100644 > > > --- a/drivers/usb/usbip/vudc_dev.c > > > +++ b/drivers/usb/usbip/vudc_dev.c > > > @@ -297,7 +297,7 @@ static void vep_free_request(struct usb_ep *_ep, struct usb_request *_req) > > > { > > > struct vrequest *req; > > > - if (WARN_ON(!_ep || !_req)) > > > + if (!_ep || !_req) > > > > It's impossible for _ep to be NULL in this callback (see > > usb_ep_free_request() for where this is called from to prove that), so I > > don't think you need to check that. It's almost impossible for _req to > > be NULL, so you might as well leave that check in. > > > > Yes. ep can never be null here in vep_free_request(). I will leave > this alone. You can drop the !_ep check at the least, no need to check something that is impossible to hit :) thanks, greg k-h
On 1/25/19 1:02 AM, Greg KH wrote: > On Tue, Jan 22, 2019 at 04:05:28PM -0700, shuah wrote: >> On 1/19/19 1:17 AM, Greg KH wrote: >>> On Fri, Jan 18, 2019 at 02:29:30PM -0700, Shuah Khan wrote: >>>> From: Shuah Khan <shuah@kernel.org> >>>> >>>> Fix vep_free_request() to return when usb_ep and usb_request are null >>>> instead of calling WARN_ON. >>>> >>>> Signed-off-by: Shuah Khan <shuah@kernel.org> >>>> --- >>>> drivers/usb/usbip/vudc_dev.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c >>>> index 1634d8698e15..bfc8218e3fb6 100644 >>>> --- a/drivers/usb/usbip/vudc_dev.c >>>> +++ b/drivers/usb/usbip/vudc_dev.c >>>> @@ -297,7 +297,7 @@ static void vep_free_request(struct usb_ep *_ep, struct usb_request *_req) >>>> { >>>> struct vrequest *req; >>>> - if (WARN_ON(!_ep || !_req)) >>>> + if (!_ep || !_req) >>> >>> It's impossible for _ep to be NULL in this callback (see >>> usb_ep_free_request() for where this is called from to prove that), so I >>> don't think you need to check that. It's almost impossible for _req to >>> be NULL, so you might as well leave that check in. >>> >> >> Yes. ep can never be null here in vep_free_request(). I will leave >> this alone. > > You can drop the !_ep check at the least, no need to check something > that is impossible to hit :) > Thanks. I will do that. -- Shuah
diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c index 1634d8698e15..bfc8218e3fb6 100644 --- a/drivers/usb/usbip/vudc_dev.c +++ b/drivers/usb/usbip/vudc_dev.c @@ -297,7 +297,7 @@ static void vep_free_request(struct usb_ep *_ep, struct usb_request *_req) { struct vrequest *req; - if (WARN_ON(!_ep || !_req)) + if (!_ep || !_req) return; req = to_vrequest(_req);