[PATCH] ubi: Fix race condition between ubi volume creation and udev

[PATCH] ubi: Fix race condition between ubi volume creation and udev

[Clay McClure]

Similar to commit 714fb87e8bc0 ("ubi: Fix race condition between ubi
device creation and udev"), we should make the volume active before
registering it.

Signed-off-by: Clay McClure <clay@daemons.net>
---
 drivers/mtd/ubi/vmt.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 85237cf661f9..3fd8d7ff7a02 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -270,6 +270,12 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
 			vol->last_eb_bytes = vol->usable_leb_size;
 	}
 
+	/* Make volume "available" before it becomes accessible via sysfs */
+	spin_lock(&ubi->volumes_lock);
+	ubi->volumes[vol_id] = vol;
+	ubi->vol_count += 1;
+	spin_unlock(&ubi->volumes_lock);
+
 	/* Register character device for the volume */
 	cdev_init(&vol->cdev, &ubi_vol_cdev_operations);
 	vol->cdev.owner = THIS_MODULE;
@@ -298,11 +304,6 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
 	if (err)
 		goto out_sysfs;
 
-	spin_lock(&ubi->volumes_lock);
-	ubi->volumes[vol_id] = vol;
-	ubi->vol_count += 1;
-	spin_unlock(&ubi->volumes_lock);
-
 	ubi_volume_notify(ubi, vol, UBI_VOLUME_ADDED);
 	self_check_volumes(ubi);
 	return err;
@@ -315,6 +316,10 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
 	 */
 	cdev_device_del(&vol->cdev, &vol->dev);
 out_mapping:
+	spin_lock(&ubi->volumes_lock);
+	ubi->volumes[vol_id] = NULL;
+	ubi->vol_count -= 1;
+	spin_unlock(&ubi->volumes_lock);
 	ubi_eba_destroy_table(eba_tbl);
 out_acc:
 	spin_lock(&ubi->volumes_lock);
-- 
2.7.4

Re: [PATCH] ubi: Fix race condition between ubi volume creation and udev

[Richard Weinberger]

On Fri, Sep 22, 2017 at 4:01 AM, Clay McClure <clay@daemons.net> wrote:
> Similar to commit 714fb87e8bc0 ("ubi: Fix race condition between ubi
> device creation and udev"), we should make the volume active before
> registering it.
>
> Signed-off-by: Clay McClure <clay@daemons.net>
> ---
>  drivers/mtd/ubi/vmt.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
> index 85237cf661f9..3fd8d7ff7a02 100644
> --- a/drivers/mtd/ubi/vmt.c
> +++ b/drivers/mtd/ubi/vmt.c
> @@ -270,6 +270,12 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
>                         vol->last_eb_bytes = vol->usable_leb_size;
>         }
>
> +       /* Make volume "available" before it becomes accessible via sysfs */
> +       spin_lock(&ubi->volumes_lock);
> +       ubi->volumes[vol_id] = vol;
> +       ubi->vol_count += 1;
> +       spin_unlock(&ubi->volumes_lock);
> +
>         /* Register character device for the volume */
>         cdev_init(&vol->cdev, &ubi_vol_cdev_operations);
>         vol->cdev.owner = THIS_MODULE;
> @@ -298,11 +304,6 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
>         if (err)
>                 goto out_sysfs;
>
> -       spin_lock(&ubi->volumes_lock);
> -       ubi->volumes[vol_id] = vol;
> -       ubi->vol_count += 1;
> -       spin_unlock(&ubi->volumes_lock);
> -
>         ubi_volume_notify(ubi, vol, UBI_VOLUME_ADDED);
>         self_check_volumes(ubi);
>         return err;
> @@ -315,6 +316,10 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
>          */
>         cdev_device_del(&vol->cdev, &vol->dev);
>  out_mapping:
> +       spin_lock(&ubi->volumes_lock);
> +       ubi->volumes[vol_id] = NULL;
> +       ubi->vol_count -= 1;
> +       spin_unlock(&ubi->volumes_lock);
>         ubi_eba_destroy_table(eba_tbl);
>  out_acc:
>         spin_lock(&ubi->volumes_lock);
> --

Uhh, yes. Thanks for catching this!

-- 
Thanks,
//richard