* [PATCH v1] tty: Register device *after* creating the cdev for a tty
@ 2025-05-26 11:25 Max Staudt
2025-05-28 7:06 ` kernel test robot
2025-05-28 8:23 ` Greg Kroah-Hartman
0 siblings, 2 replies; 4+ messages in thread
From: Max Staudt @ 2025-05-26 11:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel, Max Staudt
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 open it during this time
will lead to tty->dev being assigned NULL by alloc_tty_struct().
alloc_tty_struct() is called via tty_init_dev() when the tty is firstly
opened, and is entered with tty_mutex held, so let's lock the critical
section in tty_register_device_attr() with the same global mutex.
This guarantees that tty->dev can be assigned a sane value.
Fixes: 6a7e6f78c235 ("tty: close race between device register and open")
Signed-off-by: Max Staudt <max@enpas.org>
---
drivers/tty/tty_io.c | 59 +++++++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index ca9b7d7bad2b..94768509e2d2 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,23 +3258,7 @@ 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;
+ mutex_lock(&tty_mutex);
if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC)) {
/*
@@ -3288,19 +3273,49 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
retval = tty_cdev_add(driver, devt, index, 1);
if (retval)
- goto err_del;
+ goto err_unlock;
+
+ 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;
+
+ mutex_unlock(&tty_mutex);
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;
+ }
+
+err_unlock:
+ mutex_unlock(&tty_mutex);
+
return ERR_PTR(retval);
}
EXPORT_SYMBOL_GPL(tty_register_device_attr);
--
2.39.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1] tty: Register device *after* creating the cdev for a tty
2025-05-26 11:25 [PATCH v1] tty: Register device *after* creating the cdev for a tty Max Staudt
@ 2025-05-28 7:06 ` kernel test robot
2025-05-28 8:23 ` Greg Kroah-Hartman
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2025-05-28 7:06 UTC (permalink / raw)
To: Max Staudt
Cc: oe-lkp, lkp, linux-kernel, linux-serial, Greg Kroah-Hartman,
Jiri Slaby, Max Staudt, oliver.sang
Hello,
kernel test robot noticed "WARNING:possible_circular_locking_dependency_detected" on:
commit: c8190da3d46a287a5fe76c9f92ee041564c06236 ("[PATCH v1] tty: Register device *after* creating the cdev for a tty")
url: https://github.com/intel-lab-lkp/linux/commits/Max-Staudt/tty-Register-device-after-creating-the-cdev-for-a-tty/20250526-192806
base: https://git.kernel.org/cgit/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/all/20250526112523.23122-1-max@enpas.org/
patch subject: [PATCH v1] tty: Register device *after* creating the cdev for a tty
in testcase: boot
config: i386-randconfig-011-20250527
compiler: gcc-12
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+-------------------------------------------------------------------------------------------------------------------------------------------------------+------------+------------+
| | b495021a97 | c8190da3d4 |
+-------------------------------------------------------------------------------------------------------------------------------------------------------+------------+------------+
| WARNING:possible_circular_locking_dependency_detected | 0 | 6 |
| WARNING:possible_circular_locking_dependency_detected_swapper_is_trying_to_acquire_lock:at:tty_port_open_but_task_is_already_holding_lock:at:tty_lock | 0 | 6 |
+-------------------------------------------------------------------------------------------------------------------------------------------------------+------------+------------+
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202505281412.8c836cb7-lkp@intel.com
[ 6.990953][ T1] WARNING: possible circular locking dependency detected
[ 6.991958][ T1] 6.15.0-rc4-00107-gc8190da3d46a #1 Tainted: G T
[ 6.993098][ T1] ------------------------------------------------------
[ 6.994053][ T1] swapper/1 is trying to acquire lock:
[ 6.994811][ T1] b40b31ac (&port->mutex){+.+.}-{4:4}, at: tty_port_open (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/include/asm/bitops.h:206 kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/include/asm/bitops.h:238 kbuild/obj/consumer/i386-randconfig-011-20250527/include/asm-generic/bitops/instrumented-non-atomic.h:142 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/tty_port.h:211 kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_port.c:761)
[ 6.995978][ T1]
[ 6.995978][ T1] but task is already holding lock:
[ 6.997087][ T1] ed9fbc48 (&tty->legacy_mutex){+.+.}-{4:4}, at: tty_lock (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_mutex.c:19)
[ 7.002405][ T1]
[ 7.002405][ T1] which lock already depends on the new lock.
[ 7.002405][ T1]
[ 7.003949][ T1]
[ 7.003949][ T1] the existing dependency chain (in reverse order) is:
[ 7.005246][ T1]
[ 7.005246][ T1] -> #2 (&tty->legacy_mutex){+.+.}-{4:4}:
[ 7.006388][ T1] __lock_acquire (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:5235)
[ 7.007155][ T1] lock_acquire (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:472 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:5868)
[ 7.007870][ T1] __mutex_lock (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/include/asm/atomic.h:23 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/atomic/atomic-arch-fallback.h:457 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/jump_label.h:262 kbuild/obj/consumer/i386-randconfig-011-20250527/include/trace/events/lock.h:95 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:603 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:746)
[ 7.008582][ T1] mutex_lock_nested (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:799)
[ 7.009322][ T1] tty_lock (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_mutex.c:19)
[ 7.010008][ T1] tty_init_dev (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_io.c:1295 kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_io.c:1407)
[ 7.010739][ T1] tty_open (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_io.c:2073 kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_io.c:2120)
[ 7.011435][ T1] chrdev_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/char_dev.c:414)
[ 7.012161][ T1] do_dentry_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:956)
[ 7.012939][ T1] vfs_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:1086)
[ 7.013641][ T1] do_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/namei.c:3880)
[ 7.014340][ T1] path_openat (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/namei.c:4039)
[ 7.015052][ T1] do_filp_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/namei.c:4066)
[ 7.015775][ T1] file_open_name (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:1374)
[ 7.016565][ T1] filp_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:1393)
[ 7.017247][ T1] console_on_rootfs (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1528)
[ 7.018070][ T1] kernel_init_freeable (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1578)
[ 7.018887][ T1] kernel_init (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1459)
[ 7.019589][ T1] ret_from_fork (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/kernel/process.c:159)
[ 7.020334][ T1] ret_from_fork_asm (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/entry/entry_32.S:737)
[ 7.021115][ T1] entry_INT80_32 (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/entry/entry_32.S:942)
[ 7.021884][ T1]
[ 7.021884][ T1] -> #1 (tty_mutex){+.+.}-{4:4}:
[ 7.022934][ T1] __lock_acquire (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:5235)
[ 7.023681][ T1] lock_acquire (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:472 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:5868)
[ 7.024418][ T1] __mutex_lock (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/include/asm/atomic.h:23 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/atomic/atomic-arch-fallback.h:457 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/jump_label.h:262 kbuild/obj/consumer/i386-randconfig-011-20250527/include/trace/events/lock.h:95 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:603 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:746)
[ 7.025137][ T1] mutex_lock_nested (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:799)
[ 7.025908][ T1] tty_register_device_attr (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_io.c:3250)
[ 7.026751][ T1] tty_port_register_device_attr_serdev (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_port.c:199)
[ 7.027724][ T1] serial_core_add_one_port (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/serial_core.c:3184)
[ 7.028574][ T1] serial_core_register_port (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/serial_core.c:3388)
[ 7.029416][ T1] serial_ctrl_register_port (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/serial_ctrl.c:42)
[ 7.030245][ T1] uart_add_one_port (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/serial_port.c:144)
[ 7.031025][ T1] serial8250_register_8250_port (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/8250/8250_core.c:822)
[ 7.031940][ T1] serial_pnp_probe (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/8250/8250_pnp.c:480)
[ 7.032707][ T1] pnp_device_probe (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/pnp/driver.c:111)
[ 7.033455][ T1] really_probe (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/dd.c:579 kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/dd.c:657)
[ 7.038357][ T1] __driver_probe_device (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/dd.c:799)
[ 7.039198][ T1] driver_probe_device (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/dd.c:829)
[ 7.040004][ T1] __driver_attach (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/dd.c:1216)
[ 7.040773][ T1] bus_for_each_dev (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/bus.c:370)
[ 7.041556][ T1] driver_attach (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/dd.c:1234)
[ 7.042088][ T1] bus_add_driver (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/bus.c:678)
[ 7.042585][ T1] driver_register (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/base/driver.c:249)
[ 7.043090][ T1] pnp_register_driver (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/pnp/driver.c:281)
[ 7.043609][ T1] serial8250_pnp_init (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/8250/8250_pnp.c:531)
[ 7.044115][ T1] serial8250_init (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/8250/8250_platform.c:315)
[ 7.044602][ T1] do_one_initcall (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1257)
[ 7.045089][ T1] do_initcalls (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1318 kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1335)
[ 7.045549][ T1] kernel_init_freeable (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1569)
[ 7.046075][ T1] kernel_init (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1459)
[ 7.046531][ T1] ret_from_fork (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/kernel/process.c:159)
[ 7.047000][ T1] ret_from_fork_asm (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/entry/entry_32.S:737)
[ 7.047494][ T1] entry_INT80_32 (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/entry/entry_32.S:942)
[ 7.047972][ T1]
[ 7.047972][ T1] -> #0 (&port->mutex){+.+.}-{4:4}:
[ 7.048668][ T1] check_prev_add (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:3167)
[ 7.049154][ T1] validate_chain (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:3286 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:3909)
[ 7.049652][ T1] __lock_acquire (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:5235)
[ 7.050147][ T1] lock_acquire (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:472 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/lockdep.c:5868)
[ 7.050609][ T1] __mutex_lock (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/include/asm/atomic.h:23 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/atomic/atomic-arch-fallback.h:457 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/jump_label.h:262 kbuild/obj/consumer/i386-randconfig-011-20250527/include/trace/events/lock.h:95 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:603 kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:746)
[ 7.051064][ T1] mutex_lock_nested (kbuild/obj/consumer/i386-randconfig-011-20250527/kernel/locking/mutex.c:799)
[ 7.051547][ T1] tty_port_open (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/include/asm/bitops.h:206 kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/include/asm/bitops.h:238 kbuild/obj/consumer/i386-randconfig-011-20250527/include/asm-generic/bitops/instrumented-non-atomic.h:142 kbuild/obj/consumer/i386-randconfig-011-20250527/include/linux/tty_port.h:211 kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_port.c:761)
[ 7.052005][ T1] uart_open (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/serial/serial_core.c:1974)
[ 7.052433][ T1] tty_open (kbuild/obj/consumer/i386-randconfig-011-20250527/drivers/tty/tty_io.c:2137)
[ 7.052862][ T1] chrdev_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/char_dev.c:414)
[ 7.053311][ T1] do_dentry_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:956)
[ 7.053798][ T1] vfs_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:1086)
[ 7.054220][ T1] do_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/namei.c:3880)
[ 7.054647][ T1] path_openat (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/namei.c:4039)
[ 7.055093][ T1] do_filp_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/namei.c:4066)
[ 7.055548][ T1] file_open_name (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:1374)
[ 7.056027][ T1] filp_open (kbuild/obj/consumer/i386-randconfig-011-20250527/fs/open.c:1393)
[ 7.056453][ T1] console_on_rootfs (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1528)
[ 7.056933][ T1] kernel_init_freeable (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1578)
[ 7.057447][ T1] kernel_init (kbuild/obj/consumer/i386-randconfig-011-20250527/init/main.c:1459)
[ 7.057904][ T1] ret_from_fork (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/kernel/process.c:159)
[ 7.058365][ T1] ret_from_fork_asm (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/entry/entry_32.S:737)
[ 7.058843][ T1] entry_INT80_32 (kbuild/obj/consumer/i386-randconfig-011-20250527/arch/x86/entry/entry_32.S:942)
[ 7.059302][ T1]
[ 7.059302][ T1] other info that might help us debug this:
[ 7.059302][ T1]
[ 7.060225][ T1] Chain exists of:
[ 7.060225][ T1] &port->mutex --> tty_mutex --> &tty->legacy_mutex
[ 7.060225][ T1]
[ 7.061811][ T1] Possible unsafe locking scenario:
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250528/202505281412.8c836cb7-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] tty: Register device *after* creating the cdev for a tty
2025-05-26 11:25 [PATCH v1] tty: Register device *after* creating the cdev for a tty Max Staudt
2025-05-28 7:06 ` kernel test robot
@ 2025-05-28 8:23 ` Greg Kroah-Hartman
2025-05-28 13:31 ` Max
1 sibling, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2025-05-28 8:23 UTC (permalink / raw)
To: Max Staudt; +Cc: Jiri Slaby, linux-serial, linux-kernel
On Mon, May 26, 2025 at 08:25:23PM +0900, Max Staudt wrote:
> 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 open it during this time
> will lead to tty->dev being assigned NULL by alloc_tty_struct().
>
> alloc_tty_struct() is called via tty_init_dev() when the tty is firstly
> opened, and is entered with tty_mutex held, so let's lock the critical
> section in tty_register_device_attr() with the same global mutex.
> This guarantees that tty->dev can be assigned a sane value.
As 0-day points out, I think this adds a new locking issue :(
But it's really hard to detect this, as you are doing both a revert and
a change in the same commit. Can you make this as 2 patches, one that
does the revert which would be "easy" to review, and the second one that
does the new fix? That way we can detect what is going on easier.
> Fixes: 6a7e6f78c235 ("tty: close race between device register and open")
> Signed-off-by: Max Staudt <max@enpas.org>
You also forgot to add cc: stable on this :(
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] tty: Register device *after* creating the cdev for a tty
2025-05-28 8:23 ` Greg Kroah-Hartman
@ 2025-05-28 13:31 ` Max
0 siblings, 0 replies; 4+ messages in thread
From: Max @ 2025-05-28 13:31 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel
On 5/28/25 17:23, Greg Kroah-Hartman wrote:
> As 0-day points out, I think this adds a new locking issue :(
>
> But it's really hard to detect this, as you are doing both a revert and
> a change in the same commit. Can you make this as 2 patches, one that
> does the revert which would be "easy" to review, and the second one that
> does the new fix? That way we can detect what is going on easier.
Done!
Thanks for reviewing this, I've scratched my head about this locking for
a while, and just when I thought I had used a suitable lock... Well,
that's where maintainers (and now automation) come in :)
>> Fixes: 6a7e6f78c235 ("tty: close race between device register and open")
>> Signed-off-by: Max Staudt <max@enpas.org>
>
> You also forgot to add cc: stable on this :(
Done. I wasn't sure whether this needs to go to -stable, but since
you're the maintainer for both tty and stable, I figured you'd make a
choice anyway.
Thanks,
Max
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-05-28 13:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-26 11:25 [PATCH v1] tty: Register device *after* creating the cdev for a tty Max Staudt
2025-05-28 7:06 ` kernel test robot
2025-05-28 8:23 ` Greg Kroah-Hartman
2025-05-28 13:31 ` Max
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).