linux-kernel - Re: [GIT PULL 02/58] lightnvm: prevent bd removal if busy

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <DDBBB62C-8D94-48AB-B800-7E1D137790C3@gmail.com>
Date:   Sat, 14 Oct 2017 08:04:21 +0200
From:   Javier González <javigon.napster@...il.com>
To:     Rakesh Pandit <rakesh@...era.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Matias Bjørling <m@...rling.me>, axboe@...com,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL 02/58] lightnvm: prevent bd removal if busy


> On 13 Oct 2017, at 17.58, Javier González <javigon.napster@...il.com> wrote:
> 
> 
>>> On 13 Oct 2017, at 17.35, Rakesh Pandit <rakesh@...era.com> wrote:
>>> 
>>>> On Fri, Oct 13, 2017 at 07:58:09AM -0700, Christoph Hellwig wrote:
>>>> On Fri, Oct 13, 2017 at 02:45:51PM +0200, Matias Bjørling wrote:
>>>> From: Rakesh Pandit <rakesh@...era.com>
>>>> 
>>>> When a virtual block device is formatted and mounted after creating
>>>> with "nvme lnvm create... -t pblk", a removal from "nvm lnvm remove"
>>>> would result in this:
>>>> 
>>>> 446416.309757] bdi-block not registered
>>>> [446416.309773] ------------[ cut here ]------------
>>>> [446416.309780] WARNING: CPU: 3 PID: 4319 at fs/fs-writeback.c:2159
>>>> __mark_inode_dirty+0x268/0x340
>>>> 
>>>> Ideally removal should return -EBUSY as block device is mounted after
>>>> formatting.  This patch tries to address this checking if whole device
>>>> or any partition of it already mounted or not before removal.
>>> 
>>> How is this different from any other block device that can be
>>> removed even if a file system is mounted?
>> 
>> One can create many virtual block devices on top of physical using:
>> nvme lnvm create ... -t pblk
>> 
>> And remove them using:
>> nvme lnvm remove
>> 
>> Because the block devices are virtual in nature created by a program I was
>> expecting removal to tell me they are busy instead of bdi-block not registered
>> following by a WARNING (above).  My use case was writing automatic test case
>> but I assumed this is useful in general.
>> 
>>> 
>>>> 
>>>> Whole device is checked using "bd_super" member of block device.  This
>>>> member is always set once block device has been mounted using a
>>>> filesystem.  Another member "bd_part_count" takes care of checking any
>>>> if any partitions are under use.  "bd_part_count" is only updated
>>>> under locks when partitions are opened or closed (first open and last
>>>> release).  This at least does take care sending -EBUSY if removal is
>>>> being attempted while whole block device or any partition is mounted.
>>>> 
>>> 
>>> That's a massive layering violation, and a driver has no business
>>> looking at these fields.
>> 
>> Okay, I didn't consider this earlier.  I would suggest a revert for this.
> 
> The use case is still valid, since a block device typically does not disappear under a file system - at least not because of a script suddenly removing it by mistake. 
> 
> Any suggestion on how we can do this better?
> 

Thinking about it, it does not seem like we have any checks now when removing a fabrics block device?

Would it make sense to have a common way to let drivers know if they are in use, at least to give a warning?

Javier