Re: [PATCH] mtd: blkdevs: protect tr->devs list by mtd_table_mutex

From: Richard Weinberger <richard.weinberger@gmail.com>
To: Hou Tao <houtao1@huawei.com>
Cc: Maxim Levitsky <maximlevitsky@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	Marek Vasut <marek.vasut@gmail.com>,
	linux-mtd@lists.infradead.org,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Vignesh Raghavendra <vigneshr@ti.com>
Subject: Re: [PATCH] mtd: blkdevs: protect tr->devs list by mtd_table_mutex
Date: Tue, 15 Oct 2019 22:10:13 +0200	[thread overview]
Message-ID: <CAFLxGvwQ=4UOuscf0uuoOvckrncF2RC+rF_Pr70SpayFKfCg7Q@mail.gmail.com> (raw)
In-Reply-To: <20190929141157.115845-1-houtao1@huawei.com>

On Sun, Sep 29, 2019 at 4:05 PM Hou Tao <houtao1@huawei.com> wrote:
>
> There may be list corruption if there are concurrent list traversal
> and list deletion on tr->devs as showed in the following case:
>
> CPU 0                               CPU 1
>
> open /dev/mtdblock1
>
> // remove mtd1
> blktrans_notify_remove()
>     del_mtd_blktrans_dev()
>
> close /dev/mtdblock1
>   blktrans_release
>     blktrans_dev_put
>       acquire blktrans_ref_mutex     // remove mtd0
>       // the final release           acquire mtd_table_mutex
>       blktrans_dev_release()         blktrans_notify_remove()
>         // remove mtdblock1            // next is mtdblock1
>         list_del(&dev->list)           list_for_each_entry_safe()
>
> We could fix the problem by acquiring blktrans_ref_mutex during
> the traversal of tr->devs, but blktrans_ref_mutex needs to be released
> before invoking tr->remote_dev(), so we also need to increase the kref
> of current device else the device may be freed and decrease the kref
> after the removal.
>
> Or we could move the list deletion to del_mtd_blktrans_dev(), and protect
> the operations on tr->devs by mtd_table_mutex which has already be taken.
>
> The latter fix is simpler. We also can remove the unnecessary acquisitions
> of blktrans_ref_mutex in add_mtd_blktrans_dev() because operations on
> tr->devs have already been protected by mtd_table_mutex.
>
> Fixes: 048d87199566 ("mtd: blktrans: Hotplug fixes")
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> I found the problem by code review, and could not find a way to
> ensure the problem, because the removal of mtd devices almost
> comes from the removal of modules, and the open of /dev/mtdblockX
> will prevent the module from removing.

I'm confused. Can the problem only happen if you remove a mtd while
it is open?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/