linux-kernel - Re: Possible race condition in usb-serial.c

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: <200612222059.50652.oliver@neukum.org>
Date:	Fri, 22 Dec 2006 20:59:50 +0100
From:	Oliver Neukum <oliver@...kum.org>
To:	J <jhnlmn@...oo.com>
Cc:	Greg KH <gregkh@...e.de>, linux-usb-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: Possible race condition in usb-serial.c

Am Freitag, 22. Dezember 2006 20:08 schrieb J:
> > This problem will need some deeper surgery probably
> > involving
> > removal of the refcounting.
> 
> Refcounting may be OK if used consistently. 
> It is not OK when some pointers are ref-counted, 
> but other (in serial_table) are not (like it is
> in the current version).

No, this is a fundamental problem. You don't refcount
a pointer, you refcount a data structure. But this is
insufficient. We need to make sure the pointer points to valid
memory.
The problem with the current scheme is that serial_table
needs a lock. It needs to be taken in four places
- disconnect()
- open()
- probe()
- read_proc()

Refcounting solves only the race between disconnect() and close()
There's little use in a second locking mechanism if you use it
only in a minority of occasions.
Refcounting is a great idea if the number of references follows
a clear up -> maximum -> down -> free scheme, like for
skbs, etc..

> 
> As for the deeper surgery, what do you think about my
> earlier suggestion to start by rewriting
> usb_serial_probe
> to fully initialize usb_serial before it is added to 
> serial_table? 

Good suggestion. However, if done right, we'd go for a spin lock.

	Regards
		Oliver
-
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/