linux-kernel - Re: [PATCH] [usb-serial] fix Ooops on uplug

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: <20090721235512.7a1784be@lxorguk.ukuu.org.uk>
Date:	Tue, 21 Jul 2009 23:55:12 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Daniel Mack <daniel@...aq.de>,
	Kernel development list <linux-kernel@...r.kernel.org>,
	USB list <linux-usb@...r.kernel.org>
Subject: Re: [PATCH] [usb-serial] fix Ooops on uplug

> > We can't just skip freeing the resources in the hangup method as
> > tty_port_close_start() will return 0 and leak them on a hangup.
> 
> Wait a second.  Does the same serial_hangup() routine get called when
> an RS-232 (for example) hangup event occurs, like DCD turning off, and
> when the USB device is unplugged?  Sure, the second implies the first,
> but they need to be treated differently.  After the first, the hardware
> is still present and so the port resources shouldn't be released.

The sequences of behaviour for the tty interface are usually

	open
		allocate resources
		start uart

	do stuff

	close
		stop uart
		free resources

and if a hang up occurs:

	open
		allocate resources
		start uart
	do stuff

	hangup event
		stop uart
		free resources
		wake any pending openers

	[pending open completes]
		allocate resources
		start uart

	[original opener closes]
	close
		was hung up
		do nothing much


> Perhaps not; I'll explain.  It's very simplistic, because when I wrote 
> it I didn't know what was going on with the tty layer internals.  (I 
> still don't, come to that.)

Ok so between us we know the answer I hope 8)

> The usb_serial_port structures should exist as long as they are needed, 
> which means as long as the USB device is connected or the tty device 
> file is open.  That's how my original design was meant to work, but it 
> is now messed up by the fact that we get two "close" events -- a fake 
> one during disconnect and then the real one later.

The tty notion of "open" is really open->hangup or open->close. Once the
hangup occurs you may have a file handle to a tty object but it doesn't
talk to hardware. You need to open it again to get access again. Between
hangup and close you are sitting on a dead file handle that returns
errors if you use it. The hardware is owned by whoever opened it next.

The historical model for this is from dial in. A carrier drop is
effectively a secure attention key, it has to kill off anything left on
the port so an evil user cannot leave a key logger active on the port.

> Eliminating the fake calls seems like the cleanest solution.  
> Alternatively, we could keep the fake close (but not the fake release!)
> and change serial_close() so that it calls serial_do_free() even if 
> tty_port_close_start returns 0.

There are several other cases where tty_port_close_start returns zero
(such as multiple openers and not being the last one)

Alan
--
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/