This is a note to let you know that I've just added the patch titled Revert "tty_port: register tty ports with serdev bus" to my tty git tree which can be found at git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git in the tty-linus branch. The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.) The patch will hopefully also be merged in Linus's tree for the next -rc kernel release. If you have any questions about this process, please let me know. >From d3ba126a226a6b6da021ebfea444a2a807cde945 Mon Sep 17 00:00:00 2001 From: Johan Hovold <johan@xxxxxxxxxx> Date: Tue, 11 Apr 2017 19:07:28 +0200 Subject: Revert "tty_port: register tty ports with serdev bus" This reverts commit 8ee3fde047589dc9c201251f07d0ca1dc776feca. The new serdev bus hooked into the tty layer in tty_port_register_device() by registering a serdev controller instead of a tty device whenever a serdev client is present, and by deregistering the controller in the tty-port destructor. This is broken in several ways: Firstly, it leads to a NULL-pointer dereference whenever a tty driver later deregisters its devices as no corresponding character device will exist. Secondly, far from every tty driver uses tty-port refcounting (e.g. serial core) so the serdev devices might never be deregistered or deallocated. Thirdly, deregistering at tty-port destruction is too late as the underlying device and structures may be long gone by then. A port is not released before an open tty device is closed, something which a registered serdev client can prevent from ever happening. A driver callback while the device is gone typically also leads to crashes. Many tty drivers even keep their ports around until the driver is unloaded (e.g. serial core), something which even if a late callback never happens, leads to leaks if a device is unbound from its driver and is later rebound. The right solution here is to add a new tty_port_unregister_device() helper and to never call tty_device_unregister() whenever the port has been claimed by serdev, but since this requires modifying just about every tty driver (and multiple subsystems) it will need to be done incrementally. Reverting the offending patch is the first step in fixing the broken lifetime assumptions. A follow-up patch will add a new pair of tty-device registration helpers, which a vetted tty driver can use to support serdev (initially serial core). When every tty driver uses the serdev helpers (at least for deregistration), we can add serdev registration to tty_port_register_device() again. Note that this also fixes another issue with serdev, which currently allocates and registers a serdev controller for every tty device registered using tty_port_device_register() only to immediately deregister and deallocate it when the corresponding OF node or serdev child node is missing. This should be addressed before enabling serdev for hot-pluggable buses. Signed-off-by: Johan Hovold <johan@xxxxxxxxxx> Reviewed-by: Rob Herring <robh@xxxxxxxxxx> Cc: stable <stable@xxxxxxxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- drivers/tty/tty_port.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index 1d21a9c1d33e..0c880f17d27e 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -16,7 +16,6 @@ #include <linux/bitops.h> #include <linux/delay.h> #include <linux/module.h> -#include <linux/serdev.h> static int tty_port_default_receive_buf(struct tty_port *port, const unsigned char *p, @@ -129,15 +128,7 @@ struct device *tty_port_register_device_attr(struct tty_port *port, struct device *device, void *drvdata, const struct attribute_group **attr_grp) { - struct device *dev; - tty_port_link_device(port, driver, index); - - dev = serdev_tty_port_register(port, device, driver, index); - if (PTR_ERR(dev) != -ENODEV) - /* Skip creating cdev if we registered a serdev device */ - return dev; - return tty_register_device_attr(driver, index, device, drvdata, attr_grp); } @@ -189,9 +180,6 @@ static void tty_port_destructor(struct kref *kref) /* check if last port ref was dropped before tty release */ if (WARN_ON(port->itty)) return; - - serdev_tty_port_unregister(port); - if (port->xmit_buf) free_page((unsigned long)port->xmit_buf); tty_port_destroy(port); -- 2.13.0