linux-kernel - Re: [RFC] raise the maximum number of usb-serial devices to 512

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1306041007410.1093-100000@iolanthe.rowland.org>
Date:	Tue, 4 Jun 2013 10:13:47 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Greg KH <greg@...ah.com>
cc:	Tobias Winter <tobias@...uxdingsda.de>,
	BjÃ¸rn Mork <bjorn@...k.no>,
	Rob Landley <rob@...dley.net>, <linux-usb@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [RFC] raise the maximum number of usb-serial devices to 512

On Mon, 3 Jun 2013, Greg KH wrote:

> On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
> > But, IMHO, a nicer approach would be to make the allocation completely
> > dynamic, using e.g. the idr subsystem. Static tables are always feel
> > like straight jackets to me, no matter how big they are :)
> 
> You are right, I didn't change the code to use idr (it predates idr by
> about a decade or so), because I thought we needed the "rage" logic that
> the usb-serial minor reservation does.
> 
> But I'm not so sure anymore, so here's a patch to change to use the idr
> code, and should remove all minor number limitations (well 65k is the
> limit the tty core should be setting I think.)
> 
> Tobias, can you test this patch out?  Note, I only compiled it, did not
> get the chance to actually run it, so it might not work at all.
> 
> thanks,
> 
> greg k-h


> @@ -61,59 +62,52 @@ static LIST_HEAD(usb_serial_driver_list);
>  struct usb_serial *usb_serial_get_by_index(unsigned index)
>  {
>  	struct usb_serial *serial;
> +	struct usb_serial_port *port;
>  
>  	mutex_lock(&table_lock);
> -	serial = serial_table[index];
> -
> -	if (serial) {
> -		mutex_lock(&serial->disc_mutex);
> -		if (serial->disconnected) {
> -			mutex_unlock(&serial->disc_mutex);
> -			serial = NULL;
> -		} else {
> -			kref_get(&serial->kref);
> -		}
> -	}
> +	port = idr_find(&serial_minors, index);
>  	mutex_unlock(&table_lock);
> +	if (!port)
> +		return NULL;
> +
> +	serial = port->serial;
> +	kref_get(&serial->kref);
>  	return serial;
>  }

The test for serial->disconnected got lost.  And the locking isn't 
right; the routine is documented to return with serial->disc_mutex held 
(in the case where the device hasn't been disconnected).

Also, the kref_get() needs to occur within the scope of the table_lock.

I didn't check the rest of the patch for similar errors.  Finding three 
in the first function seemed like enough.  :-)

Alan Stern

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