linux-kernel - Re: [PATCH -next] ubi: fix race between ctrl_cdev_ioctl and ubi_cdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <decf845d-ce14-cfb7-9500-73f271b7a38e@huawei.com>
Date:   Tue, 2 Nov 2021 21:01:40 +0800
From:   Zhihao Cheng <chengzhihao1@...wei.com>
To:     Baokun Li <libaokun1@...wei.com>, <richard@....at>,
        <miquel.raynal@...tlin.com>, <vigneshr@...com>,
        <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
CC:     <patchwork@...wei.com>, <yukuai3@...wei.com>,
        Hulk Robot <hulkci@...wei.com>
Subject: Re: [PATCH -next] ubi: fix race between ctrl_cdev_ioctl and
 ubi_cdev_ioctl

在 2021/11/1 9:37, Baokun Li 写道:
Hi, Baokun
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index a7e3eb9befb6..708b1b96de01 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -467,6 +467,7 @@ static int uif_init(struct ubi_device *ubi)
>   	if (err)
>   		goto out_unreg;
>   
> +	spin_lock(&ubi->volumes_lock);
>   	for (i = 0; i < ubi->vtbl_slots; i++)
>   		if (ubi->volumes[i]) {
>   			err = ubi_add_volume(ubi, ubi->volumes[i]);
> @@ -475,11 +476,13 @@ static int uif_init(struct ubi_device *ubi)
>   				goto out_volumes;
>   			}
>   		}
> +	spin_unlock(&ubi->volumes_lock);
>   
>   	return 0;
>   
>   out_volumes:
>   	kill_volumes(ubi);
> +	spin_unlock(&ubi->volumes_lock);
>   	cdev_device_del(&ubi->cdev, &ubi->dev);
>   out_unreg:
>   	unregister_chrdev_region(ubi->cdev.dev, ubi->vtbl_slots + 1);

There still exist one place that ubi_attach_mtd_dev() may race with 
ubi_remove_volume():

ubi->bgt_thread = kthread_create(ubi_thread, ubi, "%s", ubi->bgt_name)

     goto out_debugfs;

...

out_debugfs:

     ubi_debugfs_exit_dev(ubi);

out_uif:

    uif_close(ubi);      // Another process does ioctl to remove volume, 
which may cause volume double free.