linux-kernel - Re: [RFC] cdc-acm: Fix potential deadlock (lockdep warning)

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: <20111123194423.158002cf@bob.linux.org.uk>
Date:	Wed, 23 Nov 2011 19:44:23 +0000
From:	Alan Cox <alan@...ux.intel.com>
To:	Havard Skinnemoen <hskinnemoen@...gle.com>
Cc:	Jiri Slaby <jslaby@...e.cz>, Dave Jones <davej@...hat.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Jiri Slaby <jirislaby@...il.com>
Subject: Re: [RFC] cdc-acm: Fix potential deadlock (lockdep warning)

> Ok, by USB side kref do you mean the tty_port_get/put calls I
> introduced in this patch, or a kref associated with the USB device
> itself?

The USB serial driver krefs its USB related objects too

> Ok, so the basic idea is:
>   * USB side: tty_port_get() in probe, tty_port_put() in disconnect
>   * TTY side: tty_port_get() in install, tty_port_put() in remove
> 
> And I need to check if the device is properly marked as disconnected,
> switch to vhangup, and possibly reorder things a bit as well. Right?

You shouldn't need to do the port_get in the probe or the put in the
disconnect path. The install/remove one is sufficient for all the
tty_port data because the only way a new open can occur is if the
install succeeds. So providing install and remove are suitably locked
(see the usb_serial.c code) the other cases shouldn't matter.

Any data owned by the USB part needs to be handled by the USB driver.
In the case of usb-serial it keeps its own krefs for that because the
lifetime of the port/USB serial data is not the same as the lifetime of
the tty itself.

The lifetime model is intended to be

object discovered
	Port created

	one or more iterations of
	{
		tty_install (refcounts the port and USB data)
		tty opened
		tty created

		[activity]

		tty closed final time
		tty destroyed (drops a reference on the port and USB
		data)
	}

	Port destroyed

and if the object is destroyed while the tty is opened tty_vhangup
ensures that all the linkages are broken and the port itself can be
destroyed after the last user.

That is tty_struct has a lifetime of open - close (plus a bit for any
active receive etc) while the tty_port is usually embedded in the USB
related material and has a life time of object creation to the point
that the object has gone away AND there are no ttys in existence using
it, hung up or otherwise. The tty "cleanup" hook may be useful for
that. It is called asynchronously as part of the the destructor for the
tty object.

In some ways its easier to read usb-serial.c than describe it

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