Linux 5.12 compat: inode_owner_or_capable()

[behlendorf]

Motivation and Context

Resolve Linux 5.12 compatibility build failure.

Description

The inode_owner_or_capable() function was updated in the 5.12 kernel
to support idmapped mounts. As part of this change the user namespace
is now passed as the first argument. For non-idmapped mounts, which
are currently not supported, the initial user namespace may be passed
which will result in identical behavior as before.

How Has This Been Tested?

Locally compiled against the 5.12-rc1 kernel and basic functionality
was tested.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[rkitover]

I think we have more work to do for 5.12, just tried this patch with 5.12-rc2 and I get:

checking whether xattr_handler->set() wants dentry... configure: error: 
        *** None of the expected "xattr set()" interfaces were detected.
        *** This may be because your kernel version is newer than what is
        *** supported, or you are using a patched custom kernel with
        *** incompatible modifications.
        ***
        *** ZFS Version: zfs-2.0.0-rc1_422_ge7a06356c
        *** Compatible Kernels: 3.10 - 5.11

[behlendorf]

@rkitover correct, I've converted the PR to a WIP until we can resolve the additional xattr changes.

[ckane]

So I pulled this into my tree, and it appears that the xattr_handler_get_dentry_inode test passes, but the xattr_handler_set_dentry_inode test fails with:

build/xattr_handler_set_dentry_inode/xattr_handler_set_dentry_inode.c:86:11: error:
   initialization of ‘int (*)(const struct xattr_handler *, struct user_namespace *, struct dentry *,
   struct inode *, const char *, const void *, size_t,  int)’ {aka ‘int (*)(const struct xattr_handler *,
   struct user_namespace *, struct dentry *, struct inode *, const char *, const void *,
   long unsigned int,  int)’} from incompatible pointer type ‘int (*)(const struct xattr_handler *,
   struct dentry *, struct inode *, const char *, const void *, size_t,  int)’
   {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, const void *,
   long unsigned int,  int)’} [-Werror=incompatible-pointer-types]
   86 |    .set = set,
      |           ^~~

[ckane]

Looks like the following definition was changed from 7-args to 8-args, with the struct user_namespace *mnt_userns parameter added in linux/xattr.h's struct xattr_handler:

     int (*set)(const struct xattr_handler *,
            struct user_namespace *mnt_userns, struct dentry *dentry,
            struct inode *inode, const char *name, const void *buffer,
            size_t size, int flags);

The same modification didn't happen to xattr_handler.get so that's why the current version of that test still works.

[ckane]

Change was introduced in kernel commit e65ce2a50cf6af216bea6fd80d771fcbb4c0aaa1 which also adds the following comment:

+/**
+ * posix_acl_chmod - chmod a posix acl
+ *
+ * @mnt_userns:        user namespace of the mount @inode was found from
+ * @inode:     inode to check permissions on
+ * @mode:      the new mode of @inode
+ *
+ * If the inode has been found through an idmapped mount the user namespace of
+ * the vfsmount must be passed through @mnt_userns. This function will then
+ * take care to map the inode according to @mnt_userns before checking
+ * permissions. On non-idmapped mounts or if permission checking is to be
+ * performed on the raw inode simply passs init_user_ns.
+ */

[ckane]

Looks like also these changes impact struct inode_operations in linux/fs.h as well

[ckane]

It looks like it could be a fairly large scope - so lmk if you want to dump progress onto your branch and then divide and conquer the remainder - I've pulled your 5.12 changes into my working fork

[ckane]

i've got some changes already made for covering the autoconf xattr set() error (which then brought me to mkdir() and all the other potential changes elsewhere), so I'm happy to commit that to my branch for you to pull in if you haven't done that on your local branch yet

[behlendorf]

@ckane I haven't had a chance to make any more headway on this, and I won't have the time available for a while. If you wouldn't mind taking it over this work that would be wonderful. Probably the easiest thing to do would be to add my commit to your additional work, then you can open a new PR with all the needed changes.

[ckane]

Sounds good! - I should have some time to work through this this week

[ckane]

I've merged your branch into my repo, and rebased against master, and added my changes to the pile as well: https://github.com/ckane/zfs/tree/linux-5.12-compat

I'll come in tonight and do a new PR

[behlendorf]

@ckane great, then I'll go ahead and close out this PR.