Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support idmapped mount in user namespace #14097

Merged
merged 8 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions config/kernel-iattr-vfsid.m4
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
dnl #
dnl # 6.0 API change
dnl # struct iattr has two unions for the uid and gid
dnl #
AC_DEFUN([ZFS_AC_KERNEL_SRC_IATTR_VFSID], [
ZFS_LINUX_TEST_SRC([iattr_vfsid], [
#include <linux/fs.h>
], [
struct iattr ia;
ia.ia_vfsuid = (vfsuid_t){0};
ia.ia_vfsgid = (vfsgid_t){0};
])
])

AC_DEFUN([ZFS_AC_KERNEL_IATTR_VFSID], [
AC_MSG_CHECKING([whether iattr->ia_vfsuid and iattr->ia_vfsgid exist])
ZFS_LINUX_TEST_RESULT([iattr_vfsid], [
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_IATTR_VFSID, 1,
[iattr->ia_vfsuid and iattr->ia_vfsgid exist])
],[
AC_MSG_RESULT(no)
])
])
2 changes: 2 additions & 0 deletions config/kernel.m4
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ AC_DEFUN([ZFS_AC_KERNEL_TEST_SRC], [
ZFS_AC_KERNEL_SRC___COPY_FROM_USER_INATOMIC
ZFS_AC_KERNEL_SRC_USER_NS_COMMON_INUM
ZFS_AC_KERNEL_SRC_IDMAP_MNT_API
ZFS_AC_KERNEL_SRC_IATTR_VFSID

AC_MSG_CHECKING([for available kernel interfaces])
ZFS_LINUX_TEST_COMPILE_ALL([kabi])
Expand Down Expand Up @@ -271,6 +272,7 @@ AC_DEFUN([ZFS_AC_KERNEL_TEST_RESULT], [
ZFS_AC_KERNEL___COPY_FROM_USER_INATOMIC
ZFS_AC_KERNEL_USER_NS_COMMON_INUM
ZFS_AC_KERNEL_IDMAP_MNT_API
ZFS_AC_KERNEL_IATTR_VFSID
])

dnl #
Expand Down
10 changes: 6 additions & 4 deletions include/os/linux/kernel/linux/xattr_compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ fn(const struct xattr_handler *handler, struct user_namespace *user_ns, \
struct dentry *dentry, struct inode *inode, const char *name, \
const void *buffer, size_t size, int flags) \
{ \
return (__ ## fn(inode, name, buffer, size, flags)); \
return (__ ## fn(user_ns, inode, name, buffer, size, flags)); \
}
/*
* 4.7 API change,
Expand All @@ -160,7 +160,7 @@ fn(const struct xattr_handler *handler, struct dentry *dentry, \
struct inode *inode, const char *name, const void *buffer, \
size_t size, int flags) \
{ \
return (__ ## fn(inode, name, buffer, size, flags)); \
return (__ ## fn(kcred->user_ns, inode, name, buffer, size, flags));\
}
/*
* 4.4 API change,
Expand All @@ -174,7 +174,8 @@ static int \
fn(const struct xattr_handler *handler, struct dentry *dentry, \
const char *name, const void *buffer, size_t size, int flags) \
{ \
return (__ ## fn(dentry->d_inode, name, buffer, size, flags)); \
return (__ ## fn(kcred->user_ns, dentry->d_inode, name, \
buffer, size, flags)); \
}
/*
* 2.6.33 API change,
Expand All @@ -187,7 +188,8 @@ static int \
fn(struct dentry *dentry, const char *name, const void *buffer, \
size_t size, int flags, int unused_handler_flags) \
{ \
return (__ ## fn(dentry->d_inode, name, buffer, size, flags)); \
return (__ ## fn(kcred->user_ns, dentry->d_inode, name, \
buffer, size, flags)); \
}
#else
#error "Unsupported kernel"
Expand Down
84 changes: 67 additions & 17 deletions include/os/linux/spl/sys/cred.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@

#include <linux/module.h>
#include <linux/cred.h>
#include <linux/sched.h>
#include <sys/types.h>
#include <sys/vfs.h>

typedef struct cred cred_t;

extern struct task_struct init_task;

#define kcred ((cred_t *)(init_task.cred))
#define CRED() ((cred_t *)current_cred())

Expand All @@ -45,32 +48,80 @@ typedef struct cred cred_t;
#define SGID_TO_KGID(x) (KGIDT_INIT(x))
#define KGIDP_TO_SGIDP(x) (&(x)->val)

static inline uid_t zfs_uid_into_mnt(struct user_namespace *mnt_ns, uid_t uid)
/* Check if the user ns is the initial one */
static inline boolean_t
zfs_is_init_userns(struct user_namespace *user_ns)
{
if (mnt_ns)
return (__kuid_val(make_kuid(mnt_ns, uid)));
return (uid);
#if defined(CONFIG_USER_NS)
return (user_ns == kcred->user_ns);
#else
return (B_FALSE);
#endif
}

static inline gid_t zfs_gid_into_mnt(struct user_namespace *mnt_ns, gid_t gid)
static inline struct user_namespace *zfs_i_user_ns(struct inode *inode)
{
if (mnt_ns)
return (__kgid_val(make_kgid(mnt_ns, gid)));
return (gid);
#ifdef HAVE_SUPER_USER_NS
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you support kernels without sb->s_user_ns?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we do.

return (inode->i_sb->s_user_ns);
#else
return (kcred->user_ns);
#endif
}

static inline uid_t zfs_uid_from_mnt(struct user_namespace *mnt_ns, uid_t uid)
static inline boolean_t zfs_no_idmapping(struct user_namespace *mnt_userns,
struct user_namespace *fs_userns)
{
if (mnt_ns)
return (from_kuid(mnt_ns, KUIDT_INIT(uid)));
return (uid);
return (zfs_is_init_userns(mnt_userns) || mnt_userns == fs_userns);
}

static inline gid_t zfs_gid_from_mnt(struct user_namespace *mnt_ns, gid_t gid)
static inline uid_t zfs_uid_to_vfsuid(struct user_namespace *mnt_userns,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so I understand, you're deliberately operating on plain {g,u}id_t types instead of k{g,u}id_t and vfs{g,u}id_t types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I see no need to massage around different types.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think security wise this isn't a good idea which is why we have raw {g,u}id_t for raw on-disk or userspace values, {k,g}uid_t for filesystem- or kernel-wide values, and vfs{g,u}id_t for vfs/vfsmount specific values so they can never be confused and used for one another. Just a note, not a request to change things.

struct user_namespace *fs_userns, uid_t uid)
{
if (mnt_ns)
return (from_kgid(mnt_ns, KGIDT_INIT(gid)));
return (gid);
if (zfs_no_idmapping(mnt_userns, fs_userns))
return (uid);
if (!zfs_is_init_userns(fs_userns))
uid = from_kuid(fs_userns, KUIDT_INIT(uid));
if (uid == (uid_t)-1)
return (uid);
return (__kuid_val(make_kuid(mnt_userns, uid)));
}

static inline gid_t zfs_gid_to_vfsgid(struct user_namespace *mnt_userns,
struct user_namespace *fs_userns, gid_t gid)
{
if (zfs_no_idmapping(mnt_userns, fs_userns))
return (gid);
if (!zfs_is_init_userns(fs_userns))
gid = from_kgid(fs_userns, KGIDT_INIT(gid));
if (gid == (gid_t)-1)
return (gid);
return (__kgid_val(make_kgid(mnt_userns, gid)));
}

static inline uid_t zfs_vfsuid_to_uid(struct user_namespace *mnt_userns,
struct user_namespace *fs_userns, uid_t uid)
{
if (zfs_no_idmapping(mnt_userns, fs_userns))
return (uid);
uid = from_kuid(mnt_userns, KUIDT_INIT(uid));
if (uid == (uid_t)-1)
return (uid);
if (zfs_is_init_userns(fs_userns))
return (uid);
return (__kuid_val(make_kuid(fs_userns, uid)));
}

static inline gid_t zfs_vfsgid_to_gid(struct user_namespace *mnt_userns,
struct user_namespace *fs_userns, gid_t gid)
{
if (zfs_no_idmapping(mnt_userns, fs_userns))
return (gid);
gid = from_kgid(mnt_userns, KGIDT_INIT(gid));
if (gid == (gid_t)-1)
return (gid);
if (zfs_is_init_userns(fs_userns))
return (gid);
return (__kgid_val(make_kgid(fs_userns, gid)));
}

extern void crhold(cred_t *cr);
Expand All @@ -81,5 +132,4 @@ extern gid_t crgetgid(const cred_t *cr);
extern int crgetngroups(const cred_t *cr);
extern gid_t *crgetgroups(const cred_t *cr);
extern int groupmember(gid_t gid, const cred_t *cr);

#endif /* _SPL_CRED_H */
5 changes: 3 additions & 2 deletions include/os/linux/zfs/sys/policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,14 @@ int secpolicy_vnode_create_gid(const cred_t *);
int secpolicy_vnode_remove(const cred_t *);
int secpolicy_vnode_setdac(const cred_t *, uid_t);
int secpolicy_vnode_setid_retain(struct znode *, const cred_t *, boolean_t);
int secpolicy_vnode_setids_setgids(const cred_t *, gid_t, zuserns_t *);
int secpolicy_vnode_setids_setgids(const cred_t *, gid_t, zuserns_t *,
zuserns_t *);
int secpolicy_zinject(const cred_t *);
int secpolicy_zfs(const cred_t *);
int secpolicy_zfs_proc(const cred_t *, proc_t *);
void secpolicy_setid_clear(vattr_t *, cred_t *);
int secpolicy_setid_setsticky_clear(struct inode *, vattr_t *,
const vattr_t *, cred_t *, zuserns_t *);
const vattr_t *, cred_t *, zuserns_t *, zuserns_t *);
int secpolicy_xvattr(xvattr_t *, uid_t, cred_t *, mode_t);
int secpolicy_vnode_setattr(cred_t *, struct inode *, struct vattr *,
const struct vattr *, int, int (void *, int, cred_t *), void *);
Expand Down
17 changes: 10 additions & 7 deletions module/os/linux/zfs/policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,10 @@ secpolicy_vnode_setid_retain(struct znode *zp __maybe_unused, const cred_t *cr,
* Determine that subject can set the file setgid flag.
*/
int
secpolicy_vnode_setids_setgids(const cred_t *cr, gid_t gid, zuserns_t *mnt_ns)
secpolicy_vnode_setids_setgids(const cred_t *cr, gid_t gid, zuserns_t *mnt_ns,
zuserns_t *fs_ns)
{
gid = zfs_gid_into_mnt(mnt_ns, gid);
gid = zfs_gid_to_vfsgid(mnt_ns, fs_ns, gid);
#if defined(CONFIG_USER_NS)
if (!kgid_has_mapping(cr->user_ns, SGID_TO_KGID(gid)))
return (EPERM);
Expand Down Expand Up @@ -285,9 +286,10 @@ secpolicy_setid_clear(vattr_t *vap, cred_t *cr)
* Determine that subject can set the file setid flags.
*/
static int
secpolicy_vnode_setid_modify(const cred_t *cr, uid_t owner, zuserns_t *mnt_ns)
secpolicy_vnode_setid_modify(const cred_t *cr, uid_t owner, zuserns_t *mnt_ns,
zuserns_t *fs_ns)
{
owner = zfs_uid_into_mnt(mnt_ns, owner);
owner = zfs_uid_to_vfsuid(mnt_ns, fs_ns, owner);

if (crgetuid(cr) == owner)
return (0);
Expand All @@ -313,13 +315,13 @@ secpolicy_vnode_stky_modify(const cred_t *cr)

int
secpolicy_setid_setsticky_clear(struct inode *ip, vattr_t *vap,
const vattr_t *ovap, cred_t *cr, zuserns_t *mnt_ns)
const vattr_t *ovap, cred_t *cr, zuserns_t *mnt_ns, zuserns_t *fs_ns)
{
int error;

if ((vap->va_mode & S_ISUID) != 0 &&
(error = secpolicy_vnode_setid_modify(cr,
ovap->va_uid, mnt_ns)) != 0) {
ovap->va_uid, mnt_ns, fs_ns)) != 0) {
return (error);
}

Expand All @@ -337,7 +339,8 @@ secpolicy_setid_setsticky_clear(struct inode *ip, vattr_t *vap,
* group-id bit.
*/
if ((vap->va_mode & S_ISGID) != 0 &&
secpolicy_vnode_setids_setgids(cr, ovap->va_gid, mnt_ns) != 0) {
secpolicy_vnode_setids_setgids(cr, ovap->va_gid,
mnt_ns, fs_ns) != 0) {
vap->va_mode &= ~S_ISGID;
}

Expand Down
24 changes: 15 additions & 9 deletions module/os/linux/zfs/zfs_acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1889,7 +1889,8 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr,
acl_ids->z_mode |= S_ISGID;
} else {
if ((acl_ids->z_mode & S_ISGID) &&
secpolicy_vnode_setids_setgids(cr, gid, mnt_ns) != 0) {
secpolicy_vnode_setids_setgids(cr, gid, mnt_ns,
zfs_i_user_ns(ZTOI(dzp))) != 0) {
acl_ids->z_mode &= ~S_ISGID;
}
}
Expand Down Expand Up @@ -1979,7 +1980,8 @@ zfs_getacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
if (mask == 0)
return (SET_ERROR(ENOSYS));

if ((error = zfs_zaccess(zp, ACE_READ_ACL, 0, skipaclchk, cr, NULL)))
if ((error = zfs_zaccess(zp, ACE_READ_ACL, 0, skipaclchk, cr,
kcred->user_ns)))
return (error);

mutex_enter(&zp->z_acl_lock);
Expand Down Expand Up @@ -2138,7 +2140,8 @@ zfs_setacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
if (zp->z_pflags & ZFS_IMMUTABLE)
return (SET_ERROR(EPERM));

if ((error = zfs_zaccess(zp, ACE_WRITE_ACL, 0, skipaclchk, cr, NULL)))
if ((error = zfs_zaccess(zp, ACE_WRITE_ACL, 0, skipaclchk, cr,
kcred->user_ns)))
return (error);

error = zfs_vsec_2_aclp(zfsvfs, ZTOI(zp)->i_mode, vsecp, cr, &fuidp,
Expand Down Expand Up @@ -2301,9 +2304,9 @@ zfs_zaccess_aces_check(znode_t *zp, uint32_t *working_mode,
uid_t fowner;

if (mnt_ns) {
fowner = zfs_uid_into_mnt(mnt_ns,
fowner = zfs_uid_to_vfsuid(mnt_ns, zfs_i_user_ns(ZTOI(zp)),
KUID_TO_SUID(ZTOI(zp)->i_uid));
gowner = zfs_gid_into_mnt(mnt_ns,
gowner = zfs_gid_to_vfsgid(mnt_ns, zfs_i_user_ns(ZTOI(zp)),
KGID_TO_SGID(ZTOI(zp)->i_gid));
} else
zfs_fuid_map_ids(zp, cr, &fowner, &gowner);
Expand Down Expand Up @@ -2417,7 +2420,8 @@ zfs_has_access(znode_t *zp, cred_t *cr)
{
uint32_t have = ACE_ALL_PERMS;

if (zfs_zaccess_aces_check(zp, &have, B_TRUE, cr, NULL) != 0) {
if (zfs_zaccess_aces_check(zp, &have, B_TRUE, cr,
kcred->user_ns) != 0) {
uid_t owner;

owner = zfs_fuid_map_id(ZTOZSB(zp),
Expand Down Expand Up @@ -2610,7 +2614,8 @@ zfs_fastaccesschk_execute(znode_t *zdp, cred_t *cr)
DTRACE_PROBE(zfs__fastpath__execute__access__miss);
if ((error = zfs_enter(ZTOZSB(zdp), FTAG)) != 0)
return (error);
error = zfs_zaccess(zdp, ACE_EXECUTE, 0, B_FALSE, cr, NULL);
error = zfs_zaccess(zdp, ACE_EXECUTE, 0, B_FALSE, cr,
kcred->user_ns);
zfs_exit(ZTOZSB(zdp), FTAG);
return (error);
}
Expand Down Expand Up @@ -2662,7 +2667,8 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr,
}
}

owner = zfs_uid_into_mnt(mnt_ns, KUID_TO_SUID(ZTOI(zp)->i_uid));
owner = zfs_uid_to_vfsuid(mnt_ns, zfs_i_user_ns(ZTOI(zp)),
KUID_TO_SUID(ZTOI(zp)->i_uid));
owner = zfs_fuid_map_id(ZTOZSB(zp), owner, cr, ZFS_OWNER);

/*
Expand Down Expand Up @@ -2786,7 +2792,7 @@ zfs_zaccess_unix(znode_t *zp, mode_t mode, cred_t *cr)
{
int v4_mode = zfs_unix_to_v4(mode >> 6);

return (zfs_zaccess(zp, v4_mode, 0, B_FALSE, cr, NULL));
return (zfs_zaccess(zp, v4_mode, 0, B_FALSE, cr, kcred->user_ns));
}

/* See zfs_zaccess_delete() */
Expand Down
7 changes: 4 additions & 3 deletions module/os/linux/zfs/zfs_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1113,11 +1113,11 @@ zfs_make_xattrdir(znode_t *zp, vattr_t *vap, znode_t **xzpp, cred_t *cr)
*xzpp = NULL;

if ((error = zfs_zaccess(zp, ACE_WRITE_NAMED_ATTRS, 0, B_FALSE, cr,
NULL)))
kcred->user_ns)))
return (error);

if ((error = zfs_acl_ids_create(zp, IS_XATTR, vap, cr, NULL,
&acl_ids, NULL)) != 0)
&acl_ids, kcred->user_ns)) != 0)
return (error);
if (zfs_acl_ids_overquota(zfsvfs, &acl_ids, zp->z_projid)) {
zfs_acl_ids_free(&acl_ids);
Expand Down Expand Up @@ -1265,7 +1265,8 @@ zfs_sticky_remove_access(znode_t *zdp, znode_t *zp, cred_t *cr)
cr, ZFS_OWNER);

if ((uid = crgetuid(cr)) == downer || uid == fowner ||
zfs_zaccess(zp, ACE_WRITE_DATA, 0, B_FALSE, cr, NULL) == 0)
zfs_zaccess(zp, ACE_WRITE_DATA, 0, B_FALSE, cr,
kcred->user_ns) == 0)
return (0);
else
return (secpolicy_vnode_remove(cr));
Expand Down
Loading