Re: [PATCH 01/24] USB: serial: cyberjack: fix NULL-deref at open

[Greg KH <greg@kroah.com>]

On Tue, Jan 03, 2017 at 04:39:40PM +0100, Johan Hovold wrote:
> Fix NULL-pointer dereference when clearing halt at open should the device
> lack a bulk-out endpoint.
> 
> Unable to handle kernel NULL pointer dereference at virtual address 00000030
> ...
> PC is at cyberjack_open+0x40/0x9c [cyberjack]
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/cyberjack.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
> index 5f17a3b9916d..80260b08398b 100644
> --- a/drivers/usb/serial/cyberjack.c
> +++ b/drivers/usb/serial/cyberjack.c
> @@ -50,6 +50,7 @@
>  #define CYBERJACK_PRODUCT_ID	0x0100
>  
>  /* Function prototypes */
> +static int cyberjack_attach(struct usb_serial *serial);
>  static int cyberjack_port_probe(struct usb_serial_port *port);
>  static int cyberjack_port_remove(struct usb_serial_port *port);
>  static int  cyberjack_open(struct tty_struct *tty,
> @@ -77,6 +78,7 @@ static struct usb_serial_driver cyberjack_device = {
>  	.description =		"Reiner SCT Cyberjack USB card reader",
>  	.id_table =		id_table,
>  	.num_ports =		1,
> +	.attach =		cyberjack_attach,
>  	.port_probe =		cyberjack_port_probe,
>  	.port_remove =		cyberjack_port_remove,
>  	.open =			cyberjack_open,
> @@ -100,6 +102,14 @@ struct cyberjack_private {
>  	short		wrsent;		/* Data already sent */
>  };
>  
> +static int cyberjack_attach(struct usb_serial *serial)
> +{
> +	if (serial->num_bulk_out < serial->num_ports)
> +		return -ENODEV;
> +
> +	return 0;
> +}

You end up doing much the same thing for most of these drivers, is there
any way to do it in the usb-serial core instead?

I've been playing with an idea to have a USB driver specify the number
and types of endpoints it requires and have the core just not even call
the probe function if that doesn't match up.  That should solve lots of
these issues, can't you do much the same type of thing here instead of
requiring a callback to do this?

Hm, but you want to match up the number of ports with the number of
bulk endpoint pairs.  That's tricky...

Anyway, I guess this is ok, I just get worried when I see a bunch of the
same changes in a bunch of different drivers.

thanks,

greg k-h