[PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Johan Hovold]

Make sure to check the number of endpoints to avoid dereferencing a
NULL-pointer should a malicious device lack endpoints.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/watchdog/pcwd_usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/pcwd_usb.c b/drivers/watchdog/pcwd_usb.c
index 99ebf6ea3de6..5615f4013924 100644
--- a/drivers/watchdog/pcwd_usb.c
+++ b/drivers/watchdog/pcwd_usb.c
@@ -630,6 +630,9 @@ static int usb_pcwd_probe(struct usb_interface *interface,
 		return -ENODEV;
 	}
 
+	if (iface_desc->desc.bNumEndpoints < 1)
+		return -ENODEV;
+
 	/* check out the endpoint: it has to be Interrupt & IN */
 	endpoint = &iface_desc->endpoint[0].desc;
 
-- 
2.12.0

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Guenter Roeck]

On 03/13/2017 05:49 AM, Johan Hovold wrote:
> Make sure to check the number of endpoints to avoid dereferencing a
> NULL-pointer should a malicious device lack endpoints.
>

Is this theory or was it actually observed ?

Thanks,
Guenter

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/watchdog/pcwd_usb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/watchdog/pcwd_usb.c b/drivers/watchdog/pcwd_usb.c
> index 99ebf6ea3de6..5615f4013924 100644
> --- a/drivers/watchdog/pcwd_usb.c
> +++ b/drivers/watchdog/pcwd_usb.c
> @@ -630,6 +630,9 @@ static int usb_pcwd_probe(struct usb_interface *interface,
>  		return -ENODEV;
>  	}
>
> +	if (iface_desc->desc.bNumEndpoints < 1)
> +		return -ENODEV;
> +
>  	/* check out the endpoint: it has to be Interrupt & IN */
>  	endpoint = &iface_desc->endpoint[0].desc;
>
>

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Johan Hovold]

[ Adding linux-usb which I forgot to CC for this one ]

On Mon, Mar 13, 2017 at 06:42:45AM -0700, Guenter Roeck wrote:
> On 03/13/2017 05:49 AM, Johan Hovold wrote:
> > Make sure to check the number of endpoints to avoid dereferencing a
> > NULL-pointer should a malicious device lack endpoints.
> >
> 
> Is this theory or was it actually observed ?

This was found through inspection, but creating a USB device to crash a
host with this driver enabled is easily done.

> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable <stable@vger.kernel.org>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/watchdog/pcwd_usb.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/watchdog/pcwd_usb.c b/drivers/watchdog/pcwd_usb.c
> > index 99ebf6ea3de6..5615f4013924 100644
> > --- a/drivers/watchdog/pcwd_usb.c
> > +++ b/drivers/watchdog/pcwd_usb.c
> > @@ -630,6 +630,9 @@ static int usb_pcwd_probe(struct usb_interface *interface,
> >  		return -ENODEV;
> >  	}
> >
> > +	if (iface_desc->desc.bNumEndpoints < 1)
> > +		return -ENODEV;
> > +
> >  	/* check out the endpoint: it has to be Interrupt & IN */
> >  	endpoint = &iface_desc->endpoint[0].desc;
> >
> >

Johan

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Guenter Roeck]

On Mon, Mar 13, 2017 at 03:17:39PM +0100, Johan Hovold wrote:
> [ Adding linux-usb which I forgot to CC for this one ]
> 
> On Mon, Mar 13, 2017 at 06:42:45AM -0700, Guenter Roeck wrote:
> > On 03/13/2017 05:49 AM, Johan Hovold wrote:
> > > Make sure to check the number of endpoints to avoid dereferencing a
> > > NULL-pointer should a malicious device lack endpoints.
> > >
> > 
> > Is this theory or was it actually observed ?
> 
> This was found through inspection, but creating a USB device to crash a
> host with this driver enabled is easily done.
> 
Ok, makes sense. I see other drivers doing a similar check.

Guenter

> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Cc: stable <stable@vger.kernel.org>
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >  drivers/watchdog/pcwd_usb.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/watchdog/pcwd_usb.c b/drivers/watchdog/pcwd_usb.c
> > > index 99ebf6ea3de6..5615f4013924 100644
> > > --- a/drivers/watchdog/pcwd_usb.c
> > > +++ b/drivers/watchdog/pcwd_usb.c
> > > @@ -630,6 +630,9 @@ static int usb_pcwd_probe(struct usb_interface *interface,
> > >  		return -ENODEV;
> > >  	}
> > >
> > > +	if (iface_desc->desc.bNumEndpoints < 1)
> > > +		return -ENODEV;
> > > +
> > >  	/* check out the endpoint: it has to be Interrupt & IN */
> > >  	endpoint = &iface_desc->endpoint[0].desc;
> > >
> > >
> 
> Johan

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Guenter Roeck]

On Mon, Mar 13, 2017 at 01:49:45PM +0100, Johan Hovold wrote:
> Make sure to check the number of endpoints to avoid dereferencing a
> NULL-pointer should a malicious device lack endpoints.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Note that I dropped Cc: stable from my reply since it is not appropriate
at this time.

Guenter

> ---
>  drivers/watchdog/pcwd_usb.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/watchdog/pcwd_usb.c b/drivers/watchdog/pcwd_usb.c
> index 99ebf6ea3de6..5615f4013924 100644
> --- a/drivers/watchdog/pcwd_usb.c
> +++ b/drivers/watchdog/pcwd_usb.c
> @@ -630,6 +630,9 @@ static int usb_pcwd_probe(struct usb_interface *interface,
>  		return -ENODEV;
>  	}
>  
> +	if (iface_desc->desc.bNumEndpoints < 1)
> +		return -ENODEV;
> +
>  	/* check out the endpoint: it has to be Interrupt & IN */
>  	endpoint = &iface_desc->endpoint[0].desc;
>  
> -- 
> 2.12.0
> 

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Johan Hovold]

On Mon, Mar 13, 2017 at 10:16:33AM -0700, Guenter Roeck wrote:
> On Mon, Mar 13, 2017 at 01:49:45PM +0100, Johan Hovold wrote:
> > Make sure to check the number of endpoints to avoid dereferencing a
> > NULL-pointer should a malicious device lack endpoints.
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable <stable@vger.kernel.org>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Any progress on this one? I noticed you merged it to both the fixes and
next branches in your staging tree, Guenter (but it does not show up in
linux-next). Will you be sending it on to Linus?

Thanks,
Johan

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Guenter Roeck]

On 04/03/2017 01:36 AM, Johan Hovold wrote:
> On Mon, Mar 13, 2017 at 10:16:33AM -0700, Guenter Roeck wrote:
>> On Mon, Mar 13, 2017 at 01:49:45PM +0100, Johan Hovold wrote:
>>> Make sure to check the number of endpoints to avoid dereferencing a
>>> NULL-pointer should a malicious device lack endpoints.
>>>
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>> Cc: stable <stable@vger.kernel.org>
>>> Signed-off-by: Johan Hovold <johan@kernel.org>
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> Any progress on this one? I noticed you merged it to both the fixes and
> next branches in your staging tree, Guenter (but it does not show up in
> linux-next). Will you be sending it on to Linus?
>

Hi Johan,

my watchdog staging trees are inofficial and not in linux-next.
Wim is working on setting up a new server which will provide the
official staging tree.

I asked Wim to push the pending patches. I'll do it if he asks me.

Thanks,
Guenter

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Johan Hovold]

Hi Guenter and Wim,

On Mon, Apr 03, 2017 at 07:05:46AM -0700, Guenter Roeck wrote:
> On 04/03/2017 01:36 AM, Johan Hovold wrote:
> > On Mon, Mar 13, 2017 at 10:16:33AM -0700, Guenter Roeck wrote:
> >> On Mon, Mar 13, 2017 at 01:49:45PM +0100, Johan Hovold wrote:
> >>> Make sure to check the number of endpoints to avoid dereferencing a
> >>> NULL-pointer should a malicious device lack endpoints.
> >>>
> >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >>> Cc: stable <stable@vger.kernel.org>
> >>> Signed-off-by: Johan Hovold <johan@kernel.org>
> >>
> >> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >
> > Any progress on this one? I noticed you merged it to both the fixes and
> > next branches in your staging tree, Guenter (but it does not show up in
> > linux-next). Will you be sending it on to Linus?

> my watchdog staging trees are inofficial and not in linux-next.
> Wim is working on setting up a new server which will provide the
> official staging tree.
> 
> I asked Wim to push the pending patches. I'll do it if he asks me.

I noticed Guenter's watchdog branch is now in next, but the patches for
4.12 are still not in mainline. Have you guys decided who will be
sending them on to Linus this cycle?

Thanks,
Johan

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Guenter Roeck]

On Fri, May 12, 2017 at 12:36:27PM +0200, Johan Hovold wrote:
> Hi Guenter and Wim,
> 
> On Mon, Apr 03, 2017 at 07:05:46AM -0700, Guenter Roeck wrote:
> > On 04/03/2017 01:36 AM, Johan Hovold wrote:
> > > On Mon, Mar 13, 2017 at 10:16:33AM -0700, Guenter Roeck wrote:
> > >> On Mon, Mar 13, 2017 at 01:49:45PM +0100, Johan Hovold wrote:
> > >>> Make sure to check the number of endpoints to avoid dereferencing a
> > >>> NULL-pointer should a malicious device lack endpoints.
> > >>>
> > >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > >>> Cc: stable <stable@vger.kernel.org>
> > >>> Signed-off-by: Johan Hovold <johan@kernel.org>
> > >>
> > >> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > >
> > > Any progress on this one? I noticed you merged it to both the fixes and
> > > next branches in your staging tree, Guenter (but it does not show up in
> > > linux-next). Will you be sending it on to Linus?
> 
> > my watchdog staging trees are inofficial and not in linux-next.
> > Wim is working on setting up a new server which will provide the
> > official staging tree.
> > 
> > I asked Wim to push the pending patches. I'll do it if he asks me.
> 
> I noticed Guenter's watchdog branch is now in next, but the patches for
> 4.12 are still not in mainline. Have you guys decided who will be
> sending them on to Linus this cycle?
> 
Good question. I had expected Wim to do it, since it is actually his
repository which is in linux-next. But you are correct, it isn't upstream.

Wim ?

Thanks,
Guenter

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Wim Van Sebroeck]

All,

> On Fri, May 12, 2017 at 12:36:27PM +0200, Johan Hovold wrote:
> > Hi Guenter and Wim,
> > 
> > On Mon, Apr 03, 2017 at 07:05:46AM -0700, Guenter Roeck wrote:
> > > On 04/03/2017 01:36 AM, Johan Hovold wrote:
> > > > On Mon, Mar 13, 2017 at 10:16:33AM -0700, Guenter Roeck wrote:
> > > >> On Mon, Mar 13, 2017 at 01:49:45PM +0100, Johan Hovold wrote:
> > > >>> Make sure to check the number of endpoints to avoid dereferencing a
> > > >>> NULL-pointer should a malicious device lack endpoints.
> > > >>>
> > > >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > >>> Cc: stable <stable@vger.kernel.org>
> > > >>> Signed-off-by: Johan Hovold <johan@kernel.org>
> > > >>
> > > >> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > >
> > > > Any progress on this one? I noticed you merged it to both the fixes and
> > > > next branches in your staging tree, Guenter (but it does not show up in
> > > > linux-next). Will you be sending it on to Linus?
> > 
> > > my watchdog staging trees are inofficial and not in linux-next.
> > > Wim is working on setting up a new server which will provide the
> > > official staging tree.
> > > 
> > > I asked Wim to push the pending patches. I'll do it if he asks me.
> > 
> > I noticed Guenter's watchdog branch is now in next, but the patches for
> > 4.12 are still not in mainline. Have you guys decided who will be
> > sending them on to Linus this cycle?
> > 
> Good question. I had expected Wim to do it, since it is actually his
> repository which is in linux-next. But you are correct, it isn't upstream.

Due to some issues, we (Linus, Guenter and myself) decided that Guenter will sent the pull request this time (which he allready did).

Kind regards,
Wim.

Re: [PATCH] watchdog: pcwd_usb: fix NULL-deref at probe

[Wim Van Sebroeck]

All,

> > On Fri, May 12, 2017 at 12:36:27PM +0200, Johan Hovold wrote:
> > > Hi Guenter and Wim,
> > > 
> > > On Mon, Apr 03, 2017 at 07:05:46AM -0700, Guenter Roeck wrote:
> > > > On 04/03/2017 01:36 AM, Johan Hovold wrote:
> > > > > On Mon, Mar 13, 2017 at 10:16:33AM -0700, Guenter Roeck wrote:
> > > > >> On Mon, Mar 13, 2017 at 01:49:45PM +0100, Johan Hovold wrote:
> > > > >>> Make sure to check the number of endpoints to avoid dereferencing a
> > > > >>> NULL-pointer should a malicious device lack endpoints.
> > > > >>>
> > > > >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > > >>> Cc: stable <stable@vger.kernel.org>
> > > > >>> Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > >>
> > > > >> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > > >
> > > > > Any progress on this one? I noticed you merged it to both the fixes and
> > > > > next branches in your staging tree, Guenter (but it does not show up in
> > > > > linux-next). Will you be sending it on to Linus?
> > > 
> > > > my watchdog staging trees are inofficial and not in linux-next.
> > > > Wim is working on setting up a new server which will provide the
> > > > official staging tree.
> > > > 
> > > > I asked Wim to push the pending patches. I'll do it if he asks me.
> > > 
> > > I noticed Guenter's watchdog branch is now in next, but the patches for
> > > 4.12 are still not in mainline. Have you guys decided who will be
> > > sending them on to Linus this cycle?
> > > 
> > Good question. I had expected Wim to do it, since it is actually his
> > repository which is in linux-next. But you are correct, it isn't upstream.
> 
> Due to some issues, we (Linus, Guenter and myself) decided that Guenter will sent the pull request this time (which he allready did).

OK, we were to late. We will line up the fixes for rc2. Feature changes and new drivers will be for next merge window.
My apologies to the people that have sent in these changes and drivers.

Kind regards,
Wim.