public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
From: Max Staudt <max@enpas.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>
Cc: Johan Hovold <johan@kernel.org>,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	Max Staudt <max@enpas.org>,
	stable@vger.kernel.org
Subject: [PATCH v2 1/2] tty: Register device *after* creating the cdev for a tty
Date: Wed, 28 May 2025 22:28:15 +0900	[thread overview]
Message-ID: <20250528132816.11433-1-max@enpas.org> (raw)

This change makes the tty device file available only after the tty's
backing character device is ready.

Since 6a7e6f78c235975cc14d4e141fa088afffe7062c, the class device is
registered before the cdev is created, and userspace may pick it up,
yet open() will fail because the backing cdev doesn't exist yet.
Userspace is racing the bottom half of tty_register_device_attr() here,
specifically the call to tty_cdev_add().

dev_set_uevent_suppress() was used to work around this, but this fails
on embedded systems that rely on bare devtmpfs rather than udev.
On such systems, the device file is created as part of device_add(),
and userspace can pick it up via inotify, irrespective of uevent
suppression.

So let's undo the existing patch, and create the cdev first, and only
afterwards register the class device in the kernel's device tree.

However, this restores the original race of the cdev existing before the
class device is registered, and an attempt to tty_[k]open() the chardev
between these two steps will lead to tty->dev being assigned NULL by
alloc_tty_struct().

This will be addressed in a second patch.

Fixes: 6a7e6f78c235 ("tty: close race between device register and open")
Signed-off-by: Max Staudt <max@enpas.org>
Cc: <stable@vger.kernel.org>
---
 drivers/tty/tty_io.c | 54 +++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index ca9b7d7bad2b..e922b84524d2 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3245,6 +3245,7 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 	struct ktermios *tp;
 	struct device *dev;
 	int retval;
+	bool cdev_added = false;
 
 	if (index >= driver->num) {
 		pr_err("%s: Attempt to register invalid tty line number (%d)\n",
@@ -3257,24 +3258,6 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 	else
 		tty_line_name(driver, index, name);
 
-	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		return ERR_PTR(-ENOMEM);
-
-	dev->devt = devt;
-	dev->class = &tty_class;
-	dev->parent = device;
-	dev->release = tty_device_create_release;
-	dev_set_name(dev, "%s", name);
-	dev->groups = attr_grp;
-	dev_set_drvdata(dev, drvdata);
-
-	dev_set_uevent_suppress(dev, 1);
-
-	retval = device_register(dev);
-	if (retval)
-		goto err_put;
-
 	if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC)) {
 		/*
 		 * Free any saved termios data so that the termios state is
@@ -3288,19 +3271,44 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 
 		retval = tty_cdev_add(driver, devt, index, 1);
 		if (retval)
-			goto err_del;
+			return ERR_PTR(retval);
+
+		cdev_added = true;
+	}
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		retval = -ENOMEM;
+		goto err_del_cdev;
 	}
 
-	dev_set_uevent_suppress(dev, 0);
-	kobject_uevent(&dev->kobj, KOBJ_ADD);
+	dev->devt = devt;
+	dev->class = &tty_class;
+	dev->parent = device;
+	dev->release = tty_device_create_release;
+	dev_set_name(dev, "%s", name);
+	dev->groups = attr_grp;
+	dev_set_drvdata(dev, drvdata);
+
+	retval = device_register(dev);
+	if (retval)
+		goto err_put;
 
 	return dev;
 
-err_del:
-	device_del(dev);
 err_put:
+	/*
+	 * device_register() calls device_add(), after which
+	 * we must use put_device() instead of kfree().
+	 */
 	put_device(dev);
 
+err_del_cdev:
+	if (cdev_added) {
+		cdev_del(driver->cdevs[index]);
+		driver->cdevs[index] = NULL;
+	}
+
 	return ERR_PTR(retval);
 }
 EXPORT_SYMBOL_GPL(tty_register_device_attr);
-- 
2.39.5


             reply	other threads:[~2025-05-28 13:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-28 13:28 Max Staudt [this message]
2025-05-28 13:28 ` [PATCH v2 2/2] tty: Fix race against tty_open() in tty_register_device_attr() Max Staudt
2025-06-02 10:31   ` Ilpo Järvinen
2025-06-02 13:40     ` Max Staudt
2025-06-03 13:43       ` Jiri Slaby
2025-06-03  8:49   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250528132816.11433-1-max@enpas.org \
    --to=max@enpas.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox