Re: [PATCH] Revert "fs: move the bdex_statx call to vfs_getattr_nosec"

[Christoph Hellwig <hch@lst.de>]

On Thu, Apr 24, 2025 at 11:22:40AM +0200, Christian Brauner wrote:
> On Thu, Apr 24, 2025 at 11:04:44AM +0200, Christoph Hellwig wrote:
> > On Thu, Apr 24, 2025 at 10:59:44AM +0200, Christian Brauner wrote:
> > > This reverts commit 777d0961ff95b26d5887fdae69900374364976f3.
> > > 
> > > Now that we have fixed the original issue in devtmpfs we can revert this
> > > commit because the bdev_statx() call in vfs_getattr_nosec() causes
> > > issues with the lifetime logic of dm devices.
> > 
> > Umm, no.  We need this patch.  And the devtmpfs fixes the issue that
> > it caused for devtmpfs.
> 
> For loop devices only afaict.

For anything that wants to query bdev attributes.  We are about to
add the same for nvmet, and in the future also for the scsi target
and zloop if it gets merged.

Either way the above sentence is wrong - reverting it will cause a
regression.

> The bdev_statx() implementation is
> absolutely terrifying because it triggers all that block module
> autoloading stuff and quite a few kernels still have that turned on. By
> adding that to vfs_getattr_nosec() suddenly all kernel consumers are
> able to do the same thing by accident.

I send a series to fi that yesterday.

> This already crapped devtmpfs. We
> have no idea what else it will start breaking unless you audit every
> single caller.

I don't think so.  The issue really is that devtmpfs calls this from
the block device shutdown path, and md has a really weird shutdown
path.  The first is fixed by the patch you applied, and the second is
being looked into, a series was posted about half an hour ago.

Callers that aren't directly tied into block device shutdown can't
have that issue.

> If this stays in then please figure out how to skip a call into
> blkdev_get_no_open() unless it's explicitly requested.

The problem with that is the blksize that is reported unconditionally,
for the LBS stuff.  I don't have a good answer for that as it is
reported unconditionally.  But as said, the extra bdev reference is
fairly harmless outside of magic code like devtmpfs.