Re: [PATCH] uio: fix uio_unregister_device

["gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>]

On Fri, Feb 13, 2026 at 09:47:35AM +0000, Igor Klochko (Nokia) wrote:
> When uio devices are created end removed in parallel, then we sometimes
> encounter kernel traces along the following lines:
> 
>   sysfs: cannot create duplicate filename '/class/uio/uio899'

Nice fix, but what is causing these devices to be created and removed in
parallel?  Shouldn't the initialization sequence be serialized?  And the
same with removal?

> 
> which stem from:
> 
>   sysfs_create_link+0x24/0x50
>   device_add+0x2f0/0x780
>   __uio_register_device+0x18c/0x550
> 
> The sysfs directory creation is performed synchronously as part of the
> device_add call. The high level sequence for uio registration is:
> 
>   1. uio_get_minor (idr call, in critical section)
>   2. device_add (leads to sysfs directory)
>   3. manage attributes (popuplates part of the sysfs directory)
> 
> For unregistration we have by default the following flow:
> 
>   1. clean-up attributes
>   2. uio_free_minor (idr call, in critical section)
>   3. device_unregister (cleans up sysfs directory)
> 
> This creates a racing problem when we are in parallel creating and
> removing uio devices. The uio-minor that is freed when calling uio_free_minor can be
> claimed by a subsequent uio_get_minor call. The problem is that the device_add
> flow can end up triggered, leading to a sysfs directory creation; while the
> device_unregister flow has not yet cleaned up the sysfs directory.

Nit, line wrapping at the same column width :)

> 
> This patch cleans up this problem by mirroring the registration and
> unregistration
> flow correctly. After this patch, the unregistration flow becomes:
> 
>   1. clean-up attributes
>   2. device_unregister
>   3. uio_free_minor
> 
> Fixes: 0c9ae0b86 ("uio: Fix use-after-free in uio_open")

Please use more digits, as the documentation mentions.


> Signed-off-by: Philippe Belet <philippe.belet@nokia.com>
> Reviewed-by: Igor Klochko <igor.klochko@nokia.com>
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index fa0d4e6aee16..5dd137a85576 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c 
> @@ -1125,8 +1125,8 @@ void uio_unregister_device(struct uio_info *info)
>         wake_up_interruptible(&idev->wait);
>         kill_fasync(&idev->async_queue, SIGIO, POLL_HUP);
> 
> -       uio_free_minor(minor);
>         device_unregister(&idev->dev);
> +       uio_free_minor(minor);

So no locking is needed here?  It's only the minor that is getting
messed up?

And should this be cc: stable?

thanks,

greg k-h