Re: [PATCH v1] tty: Register device *after* creating the cdev for a tty

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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