mbox series

[v4,00/21] btrfs: support idmapped mounts

Message ID 20210727104900.829215-1-brauner@kernel.org (mailing list archive)
Headers show
Series btrfs: support idmapped mounts | expand

Message

Christian Brauner July 27, 2021, 10:48 a.m. UTC
From: Christian Brauner <christian.brauner@ubuntu.com>

Hey everyone,

/* v4 */
Rename new helper to lookup_one() and add new Reviewed-bys.

This series enables the creation of idmapped mounts on btrfs. On the list of
filesystems btrfs was pretty high-up and requested quite often from userspace
(cf. [1]). This series requires just a few changes to the vfs for specific
lookup helpers that btrfs relies on to perform permission checking when looking
up an inode. The changes are required to port some other filesystem as well.

The conversion of the necessary btrfs internals was fairly straightforward. No
invasive changes were needed. I've decided to split up the patchset into very
small individual patches. This hopefully makes the series more readable and
fairly easy to review. The overall changeset is quite small.

All non-filesystem wide ioctls that peform permission checking based on inodes
can be supported on idmapped mounts. There are really just a few restrictions.
This should really only affect the deletion of subvolumes by subvolume id which
can be used to delete any subvolume in the filesystem even though the caller
might not even be able to see the subvolume under their mount. Other than that
behavior on idmapped and non-idmapped mounts is identical for all enabled
ioctls. People interested in idmappings on idmapped mounts should read [2].

The changeset has an associated new testsuite specific to btrfs. The
core vfs operations that btrfs implements are covered by the generic
idmapped mount testsuite. For the ioctls a new testsuite was added. It
is sent alongside this patchset for ease of review but will very likely
be merged independent of it.

All patches are based on v5.14-rc3.

The series can be pulled from:
https://git.kernel.org/brauner/h/fs.idmapped.btrfs
https://github.com/brauner/linux/tree/fs.idmapped.btrfs

The xfstests can be pulled from:
https://git.kernel.org/brauner/xfstests-dev/h/fs.idmapped.btrfs
https://github.com/brauner/xfstests/tree/fs.idmapped.btrfs

Note, the new btrfs xfstests patch is on top of a branch of mine
containing a few more preliminary patches. So if you want to run the
tests, please simply pull the branch and build from there.

The series has been tested with xfstests including the newly added btrfs
specific test. All tests pass.
There were three unrelated failures that I observed: btrfs/219,
btrfs/2020 and btrfs/235. All three also fail on earlier kernels
without the patch series applied.

Thanks!
Christian

[1]: https://github.com/systemd/systemd/pull/19438#discussion_r622807165
[2]: https://lore.kernel.org/linux-fsdevel/20210727104416.828293-1-brauner@kernel.org

Summary changelog. Please individual patches for full changelogs.

/* v3 */
- base: v5.14-rc3
- Added Josef's Reviewed-by's (Thank you!)
- Switched subvolume/snapshot deletion error code (David)

Christian Brauner (20):
  namei: add mapping aware lookup helper
  btrfs/inode: handle idmaps in btrfs_new_inode()
  btrfs/inode: allow idmapped rename iop
  btrfs/inode: allow idmapped getattr iop
  btrfs/inode: allow idmapped mknod iop
  btrfs/inode: allow idmapped create iop
  btrfs/inode: allow idmapped mkdir iop
  btrfs/inode: allow idmapped symlink iop
  btrfs/inode: allow idmapped tmpfile iop
  btrfs/inode: allow idmapped setattr iop
  btrfs/inode: allow idmapped permission iop
  btrfs/ioctl: check whether fs{g,u}id are mapped during subvolume
    creation
  btrfs/inode: allow idmapped BTRFS_IOC_{SNAP,SUBVOL}_CREATE{_V2} ioctl
  btrfs/ioctl: allow idmapped BTRFS_IOC_SNAP_DESTROY{_V2} ioctl
  btrfs/ioctl: relax restrictions for BTRFS_IOC_SNAP_DESTROY_V2 with
    subvolids
  btrfs/ioctl: allow idmapped BTRFS_IOC_SET_RECEIVED_SUBVOL{_32} ioctl
  btrfs/ioctl: allow idmapped BTRFS_IOC_SUBVOL_SETFLAGS ioctl
  btrfs/ioctl: allow idmapped BTRFS_IOC_INO_LOOKUP_USER ioctl
  btrfs/acl: handle idmapped mounts
  btrfs/super: allow idmapped btrfs

 fs/btrfs/acl.c        | 11 ++---
 fs/btrfs/ctree.h      |  3 +-
 fs/btrfs/inode.c      | 62 +++++++++++++++-------------
 fs/btrfs/ioctl.c      | 94 ++++++++++++++++++++++++++++---------------
 fs/btrfs/super.c      |  2 +-
 fs/namei.c            | 43 +++++++++++++++++---
 include/linux/namei.h |  2 +
 7 files changed, 145 insertions(+), 72 deletions(-)


base-commit: ff1176468d368232b684f75e82563369208bc371

Comments

Christian Brauner Aug. 2, 2021, 12:28 p.m. UTC | #1
On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
> 
> Hey everyone,
> 
> /* v4 */
> Rename new helper to lookup_one() and add new Reviewed-bys.
> 
> This series enables the creation of idmapped mounts on btrfs. On the list of
> filesystems btrfs was pretty high-up and requested quite often from userspace
> (cf. [1]). This series requires just a few changes to the vfs for specific
> lookup helpers that btrfs relies on to perform permission checking when looking
> up an inode. The changes are required to port some other filesystem as well.
> 
> The conversion of the necessary btrfs internals was fairly straightforward. No
> invasive changes were needed. I've decided to split up the patchset into very
> small individual patches. This hopefully makes the series more readable and
> fairly easy to review. The overall changeset is quite small.
> 
> All non-filesystem wide ioctls that peform permission checking based on inodes
> can be supported on idmapped mounts. There are really just a few restrictions.
> This should really only affect the deletion of subvolumes by subvolume id which
> can be used to delete any subvolume in the filesystem even though the caller
> might not even be able to see the subvolume under their mount. Other than that
> behavior on idmapped and non-idmapped mounts is identical for all enabled
> ioctls. People interested in idmappings on idmapped mounts should read [2].
> 
> The changeset has an associated new testsuite specific to btrfs. The
> core vfs operations that btrfs implements are covered by the generic
> idmapped mount testsuite. For the ioctls a new testsuite was added. It
> is sent alongside this patchset for ease of review but will very likely
> be merged independent of it.
> 
> All patches are based on v5.14-rc3.
> 
> The series can be pulled from:
> https://git.kernel.org/brauner/h/fs.idmapped.btrfs
> https://github.com/brauner/linux/tree/fs.idmapped.btrfs
> 
> The xfstests can be pulled from:
> https://git.kernel.org/brauner/xfstests-dev/h/fs.idmapped.btrfs
> https://github.com/brauner/xfstests/tree/fs.idmapped.btrfs
> 
> Note, the new btrfs xfstests patch is on top of a branch of mine
> containing a few more preliminary patches. So if you want to run the
> tests, please simply pull the branch and build from there.
> 
> The series has been tested with xfstests including the newly added btrfs
> specific test. All tests pass.
> There were three unrelated failures that I observed: btrfs/219,
> btrfs/2020 and btrfs/235. All three also fail on earlier kernels
> without the patch series applied.

Hey David,

Sorry to ping, could I answer the outstanding questions you had and are
you okay with this series?

Christian
Christian Brauner Aug. 9, 2021, 10:12 a.m. UTC | #2
On Mon, Aug 02, 2021 at 02:28:27PM +0200, Christian Brauner wrote:
> On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> > 
> > Hey everyone,
> > 
> > /* v4 */
> > Rename new helper to lookup_one() and add new Reviewed-bys.
> > 
> > This series enables the creation of idmapped mounts on btrfs. On the list of
> > filesystems btrfs was pretty high-up and requested quite often from userspace
> > (cf. [1]). This series requires just a few changes to the vfs for specific
> > lookup helpers that btrfs relies on to perform permission checking when looking
> > up an inode. The changes are required to port some other filesystem as well.
> > 
> > The conversion of the necessary btrfs internals was fairly straightforward. No
> > invasive changes were needed. I've decided to split up the patchset into very
> > small individual patches. This hopefully makes the series more readable and
> > fairly easy to review. The overall changeset is quite small.
> > 
> > All non-filesystem wide ioctls that peform permission checking based on inodes
> > can be supported on idmapped mounts. There are really just a few restrictions.
> > This should really only affect the deletion of subvolumes by subvolume id which
> > can be used to delete any subvolume in the filesystem even though the caller
> > might not even be able to see the subvolume under their mount. Other than that
> > behavior on idmapped and non-idmapped mounts is identical for all enabled
> > ioctls. People interested in idmappings on idmapped mounts should read [2].
> > 
> > The changeset has an associated new testsuite specific to btrfs. The
> > core vfs operations that btrfs implements are covered by the generic
> > idmapped mount testsuite. For the ioctls a new testsuite was added. It
> > is sent alongside this patchset for ease of review but will very likely
> > be merged independent of it.
> > 
> > All patches are based on v5.14-rc3.
> > 
> > The series can be pulled from:
> > https://git.kernel.org/brauner/h/fs.idmapped.btrfs
> > https://github.com/brauner/linux/tree/fs.idmapped.btrfs
> > 
> > The xfstests can be pulled from:
> > https://git.kernel.org/brauner/xfstests-dev/h/fs.idmapped.btrfs
> > https://github.com/brauner/xfstests/tree/fs.idmapped.btrfs
> > 
> > Note, the new btrfs xfstests patch is on top of a branch of mine
> > containing a few more preliminary patches. So if you want to run the
> > tests, please simply pull the branch and build from there.
> > 
> > The series has been tested with xfstests including the newly added btrfs
> > specific test. All tests pass.
> > There were three unrelated failures that I observed: btrfs/219,
> > btrfs/2020 and btrfs/235. All three also fail on earlier kernels
> > without the patch series applied.
> 
> Hey David,
> 
> Sorry to ping, could I answer the outstanding questions you had and are
> you okay with this series?

I stuffed the lookup helper (and nothing else) this btrfs conversion
series needs into my tree and provided it under the tag
fs.idmapped.v5.15
at
git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git
and merged it into my for-next.

Christian
David Sterba Aug. 9, 2021, 2:44 p.m. UTC | #3
On Mon, Aug 02, 2021 at 02:28:27PM +0200, Christian Brauner wrote:
> On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> > 
> > Hey everyone,
> > 
> > /* v4 */
> > Rename new helper to lookup_one() and add new Reviewed-bys.
> > 
> > This series enables the creation of idmapped mounts on btrfs. On the list of
> > filesystems btrfs was pretty high-up and requested quite often from userspace
> > (cf. [1]). This series requires just a few changes to the vfs for specific
> > lookup helpers that btrfs relies on to perform permission checking when looking
> > up an inode. The changes are required to port some other filesystem as well.
> > 
> > The conversion of the necessary btrfs internals was fairly straightforward. No
> > invasive changes were needed. I've decided to split up the patchset into very
> > small individual patches. This hopefully makes the series more readable and
> > fairly easy to review. The overall changeset is quite small.
> > 
> > All non-filesystem wide ioctls that peform permission checking based on inodes
> > can be supported on idmapped mounts. There are really just a few restrictions.
> > This should really only affect the deletion of subvolumes by subvolume id which
> > can be used to delete any subvolume in the filesystem even though the caller
> > might not even be able to see the subvolume under their mount. Other than that
> > behavior on idmapped and non-idmapped mounts is identical for all enabled
> > ioctls. People interested in idmappings on idmapped mounts should read [2].
> > 
> > The changeset has an associated new testsuite specific to btrfs. The
> > core vfs operations that btrfs implements are covered by the generic
> > idmapped mount testsuite. For the ioctls a new testsuite was added. It
> > is sent alongside this patchset for ease of review but will very likely
> > be merged independent of it.
> > 
> > All patches are based on v5.14-rc3.
> > 
> > The series can be pulled from:
> > https://git.kernel.org/brauner/h/fs.idmapped.btrfs
> > https://github.com/brauner/linux/tree/fs.idmapped.btrfs
> > 
> > The xfstests can be pulled from:
> > https://git.kernel.org/brauner/xfstests-dev/h/fs.idmapped.btrfs
> > https://github.com/brauner/xfstests/tree/fs.idmapped.btrfs
> > 
> > Note, the new btrfs xfstests patch is on top of a branch of mine
> > containing a few more preliminary patches. So if you want to run the
> > tests, please simply pull the branch and build from there.
> > 
> > The series has been tested with xfstests including the newly added btrfs
> > specific test. All tests pass.
> > There were three unrelated failures that I observed: btrfs/219,
> > btrfs/2020 and btrfs/235. All three also fail on earlier kernels
> > without the patch series applied.
> 
> Hey David,
> 
> Sorry to ping, could I answer the outstanding questions you had and are
> you okay with this series?

I'll need to do one more pass but I don't remember any big issues or
anything that couldn't be adjusted later. This is going to be the last
nontrivial patchset, time is almost up for next merge window code
freeze.

Patch 1, the VFS part, does not have acks but for inclusion into
for-next I don't think it's necessary. I'll let you know once I push the
idmap branch to for-next so you can drop the patch.
Christian Brauner Aug. 9, 2021, 3:20 p.m. UTC | #4
On Mon, Aug 09, 2021 at 04:44:39PM +0200, David Sterba wrote:
> On Mon, Aug 02, 2021 at 02:28:27PM +0200, Christian Brauner wrote:
> > On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> > > From: Christian Brauner <christian.brauner@ubuntu.com>
> > > 
> > > Hey everyone,
> > > 
> > > /* v4 */
> > > Rename new helper to lookup_one() and add new Reviewed-bys.
> > > 
> > > This series enables the creation of idmapped mounts on btrfs. On the list of
> > > filesystems btrfs was pretty high-up and requested quite often from userspace
> > > (cf. [1]). This series requires just a few changes to the vfs for specific
> > > lookup helpers that btrfs relies on to perform permission checking when looking
> > > up an inode. The changes are required to port some other filesystem as well.
> > > 
> > > The conversion of the necessary btrfs internals was fairly straightforward. No
> > > invasive changes were needed. I've decided to split up the patchset into very
> > > small individual patches. This hopefully makes the series more readable and
> > > fairly easy to review. The overall changeset is quite small.
> > > 
> > > All non-filesystem wide ioctls that peform permission checking based on inodes
> > > can be supported on idmapped mounts. There are really just a few restrictions.
> > > This should really only affect the deletion of subvolumes by subvolume id which
> > > can be used to delete any subvolume in the filesystem even though the caller
> > > might not even be able to see the subvolume under their mount. Other than that
> > > behavior on idmapped and non-idmapped mounts is identical for all enabled
> > > ioctls. People interested in idmappings on idmapped mounts should read [2].
> > > 
> > > The changeset has an associated new testsuite specific to btrfs. The
> > > core vfs operations that btrfs implements are covered by the generic
> > > idmapped mount testsuite. For the ioctls a new testsuite was added. It
> > > is sent alongside this patchset for ease of review but will very likely
> > > be merged independent of it.
> > > 
> > > All patches are based on v5.14-rc3.
> > > 
> > > The series can be pulled from:
> > > https://git.kernel.org/brauner/h/fs.idmapped.btrfs
> > > https://github.com/brauner/linux/tree/fs.idmapped.btrfs
> > > 
> > > The xfstests can be pulled from:
> > > https://git.kernel.org/brauner/xfstests-dev/h/fs.idmapped.btrfs
> > > https://github.com/brauner/xfstests/tree/fs.idmapped.btrfs
> > > 
> > > Note, the new btrfs xfstests patch is on top of a branch of mine
> > > containing a few more preliminary patches. So if you want to run the
> > > tests, please simply pull the branch and build from there.
> > > 
> > > The series has been tested with xfstests including the newly added btrfs
> > > specific test. All tests pass.
> > > There were three unrelated failures that I observed: btrfs/219,
> > > btrfs/2020 and btrfs/235. All three also fail on earlier kernels
> > > without the patch series applied.
> > 
> > Hey David,
> > 
> > Sorry to ping, could I answer the outstanding questions you had and are
> > you okay with this series?
> 
> I'll need to do one more pass but I don't remember any big issues or
> anything that couldn't be adjusted later. This is going to be the last
> nontrivial patchset, time is almost up for next merge window code
> freeze.
> 
> Patch 1, the VFS part, does not have acks but for inclusion into
> for-next I don't think it's necessary. I'll let you know once I push the
> idmap branch to for-next so you can drop the patch.

Ok, sounds good. I wasn't sure if you wanted to base your branch on the
tag or just carry it yourself. Whatever works best.

Christian
David Sterba Aug. 11, 2021, 10:12 a.m. UTC | #5
On Mon, Aug 09, 2021 at 05:20:12PM +0200, Christian Brauner wrote:
> On Mon, Aug 09, 2021 at 04:44:39PM +0200, David Sterba wrote:
> > On Mon, Aug 02, 2021 at 02:28:27PM +0200, Christian Brauner wrote:
> > > On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> > I'll need to do one more pass but I don't remember any big issues or
> > anything that couldn't be adjusted later. This is going to be the last
> > nontrivial patchset, time is almost up for next merge window code
> > freeze.
> > 
> > Patch 1, the VFS part, does not have acks but for inclusion into
> > for-next I don't think it's necessary. I'll let you know once I push the
> > idmap branch to for-next so you can drop the patch.
> 
> Ok, sounds good. I wasn't sure if you wanted to base your branch on the
> tag or just carry it yourself. Whatever works best.

The branch is now as topic branch in my for-next and has been pushed, so
there could be a minor conflict in the VFS patch in linux-next (I've
removed the extern keyword as Christoph pointed out).
Christian Brauner Aug. 11, 2021, 10:43 a.m. UTC | #6
On Wed, Aug 11, 2021 at 12:12:15PM +0200, David Sterba wrote:
> On Mon, Aug 09, 2021 at 05:20:12PM +0200, Christian Brauner wrote:
> > On Mon, Aug 09, 2021 at 04:44:39PM +0200, David Sterba wrote:
> > > On Mon, Aug 02, 2021 at 02:28:27PM +0200, Christian Brauner wrote:
> > > > On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> > > I'll need to do one more pass but I don't remember any big issues or
> > > anything that couldn't be adjusted later. This is going to be the last
> > > nontrivial patchset, time is almost up for next merge window code
> > > freeze.
> > > 
> > > Patch 1, the VFS part, does not have acks but for inclusion into
> > > for-next I don't think it's necessary. I'll let you know once I push the
> > > idmap branch to for-next so you can drop the patch.
> > 
> > Ok, sounds good. I wasn't sure if you wanted to base your branch on the
> > tag or just carry it yourself. Whatever works best.
> 
> The branch is now as topic branch in my for-next and has been pushed, so
> there could be a minor conflict in the VFS patch in linux-next (I've
> removed the extern keyword as Christoph pointed out).

Thank you. I can drop the VFS patch then and let you carry it, right?

Christian
David Sterba Aug. 11, 2021, 10:52 a.m. UTC | #7
On Wed, Aug 11, 2021 at 12:43:39PM +0200, Christian Brauner wrote:
> On Wed, Aug 11, 2021 at 12:12:15PM +0200, David Sterba wrote:
> > On Mon, Aug 09, 2021 at 05:20:12PM +0200, Christian Brauner wrote:
> > > On Mon, Aug 09, 2021 at 04:44:39PM +0200, David Sterba wrote:
> > > > On Mon, Aug 02, 2021 at 02:28:27PM +0200, Christian Brauner wrote:
> > > > > On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> > > > I'll need to do one more pass but I don't remember any big issues or
> > > > anything that couldn't be adjusted later. This is going to be the last
> > > > nontrivial patchset, time is almost up for next merge window code
> > > > freeze.
> > > > 
> > > > Patch 1, the VFS part, does not have acks but for inclusion into
> > > > for-next I don't think it's necessary. I'll let you know once I push the
> > > > idmap branch to for-next so you can drop the patch.
> > > 
> > > Ok, sounds good. I wasn't sure if you wanted to base your branch on the
> > > tag or just carry it yourself. Whatever works best.
> > 
> > The branch is now as topic branch in my for-next and has been pushed, so
> > there could be a minor conflict in the VFS patch in linux-next (I've
> > removed the extern keyword as Christoph pointed out).
> 
> Thank you. I can drop the VFS patch then and let you carry it, right?

Yes, thanks.
Christian Brauner Aug. 11, 2021, 1:29 p.m. UTC | #8
On Wed, Aug 11, 2021 at 12:52:55PM +0200, David Sterba wrote:
> On Wed, Aug 11, 2021 at 12:43:39PM +0200, Christian Brauner wrote:
> > On Wed, Aug 11, 2021 at 12:12:15PM +0200, David Sterba wrote:
> > > On Mon, Aug 09, 2021 at 05:20:12PM +0200, Christian Brauner wrote:
> > > > On Mon, Aug 09, 2021 at 04:44:39PM +0200, David Sterba wrote:
> > > > > On Mon, Aug 02, 2021 at 02:28:27PM +0200, Christian Brauner wrote:
> > > > > > On Tue, Jul 27, 2021 at 12:48:39PM +0200, Christian Brauner wrote:
> > > > > I'll need to do one more pass but I don't remember any big issues or
> > > > > anything that couldn't be adjusted later. This is going to be the last
> > > > > nontrivial patchset, time is almost up for next merge window code
> > > > > freeze.
> > > > > 
> > > > > Patch 1, the VFS part, does not have acks but for inclusion into
> > > > > for-next I don't think it's necessary. I'll let you know once I push the
> > > > > idmap branch to for-next so you can drop the patch.
> > > > 
> > > > Ok, sounds good. I wasn't sure if you wanted to base your branch on the
> > > > tag or just carry it yourself. Whatever works best.
> > > 
> > > The branch is now as topic branch in my for-next and has been pushed, so
> > > there could be a minor conflict in the VFS patch in linux-next (I've
> > > removed the extern keyword as Christoph pointed out).
> > 
> > Thank you. I can drop the VFS patch then and let you carry it, right?
> 
> Yes, thanks.

Dropped, thanks!