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.

Sign up for GitHub

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

[RFC] Remove znode's z_uid/z_gid member #4685

Closed
wants to merge 2 commits into from
Closed

[RFC] Remove znode's z_uid/z_gid member #4685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fe322f
Check whether the kernel supports i_uid/gid_read/write helpers
May 30, 2016
49366b2
Remove znode's z_uid/z_gid member
May 22, 2016
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
22 changes: 22 additions & 0 deletions config/kernel-kuid-helpers.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
dnl #
dnl # 3.5 API change,
dnl # Since usernamespaces were introduced in kernel version 3.5, it
dnl # became necessary to go through one more level of indirection
dnl # when dealing with uid/gid - namely the kuid type.
dnl #
dnl #
AC_DEFUN([ZFS_AC_KERNEL_KUID_HELPERS], [
AC_MSG_CHECKING([whether i_(uid|gid)_(read|write) exist])
ZFS_LINUX_TRY_COMPILE([
#include <linux/fs.h>
],[
struct inode *ip = NULL;
(void) i_uid_read(ip);
],[
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_KUID_HELPERS, 1,
[i_(uid|gid)_(read|write) exist])
],[
AC_MSG_RESULT(no)
])
])
1 change: 1 addition & 0 deletions config/kernel.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ AC_DEFUN([ZFS_AC_CONFIG_KERNEL], [
ZFS_AC_KERNEL_MAKE_REQUEST_FN
ZFS_AC_KERNEL_GENERIC_IO_ACCT
ZFS_AC_KERNEL_FPU
ZFS_AC_KERNEL_KUID_HELPERS

AS_IF([test "$LINUX_OBJ" != "$LINUX"], [
KERNELMAKE_PARAMS="$KERNELMAKE_PARAMS O=$LINUX_OBJ"
Expand Down
53 changes: 53 additions & 0 deletions include/linux/vfs_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#define _ZFS_VFS_H

#include <sys/taskq.h>
#include <sys/cred.h>
#include <linux/backing-dev.h>

/*
Expand Down Expand Up @@ -352,6 +353,58 @@ static inline struct inode *file_inode(const struct file *f)
}
#endif /* HAVE_FILE_INODE */

#ifdef HAVE_KUID_HELPERS
static inline uid_t zfs_uid_read_impl(struct inode *ip)
{
return (from_kuid(kcred->user_ns, ip->i_uid));
}

static inline uid_t zfs_uid_read(struct inode *ip)
{
return (zfs_uid_read_impl(ip));
}

static inline gid_t zfs_gid_read_impl(struct inode *ip)
{
return (from_kgid(kcred->user_ns, ip->i_gid));
}

static inline gid_t zfs_gid_read(struct inode *ip)
{
return (zfs_gid_read_impl(ip));
}

static inline void zfs_uid_write(struct inode *ip, uid_t uid)
{
ip->i_uid = make_kuid(kcred->user_ns, uid);
}

static inline void zfs_gid_write(struct inode *ip, gid_t gid)
{
ip->i_gid = make_kgid(kcred->user_ns, gid);
}
#else
static inline uid_t zfs_uid_read(struct inode *ip)
{
return (ip->i_uid);
}

static inline gid_t zfs_gid_read(struct inode *ip)
{
return (ip->i_gid);
}

static inline void zfs_uid_write(struct inode *ip, uid_t uid)
{
ip->i_uid = uid;
}

static inline void zfs_gid_write(struct inode *ip, gid_t gid)
{
ip->i_gid = gid;
}
#endif

/*
* 2.6.38 API change
*/
Expand Down
31 changes: 16 additions & 15 deletions include/sys/trace_acl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#define _TRACE_ACL_H

#include <linux/tracepoint.h>
#include <linux/vfs_compat.h>
#include <sys/types.h>

/*
Expand All @@ -56,15 +57,15 @@ DECLARE_EVENT_CLASS(zfs_ace_class,
__field(uint64_t, z_mapcnt)
__field(uint64_t, z_size)
__field(uint64_t, z_pflags)
__field(uint64_t, z_uid)
__field(uint64_t, z_gid)
__field(uint32_t, z_sync_cnt)
__field(mode_t, z_mode)
__field(boolean_t, z_is_sa)
__field(boolean_t, z_is_mapped)
__field(boolean_t, z_is_ctldir)
__field(boolean_t, z_is_stale)

__field(uint16_t, i_uid)
__field(uint16_t, i_gid)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these both be uint32_t?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. The reason Why I thought this should be uint16 is that the overflowuid is set to be 65535.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a matter of fact they should. The reason why I put them to uint16 is because the overflowuid/gid sysctls are set to 65535. Would you care to fix this in the merge or should I send an updated version ?

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as you agree it's just a uint16_t/uint32_t I can fix it in the merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, those uint16 should be turned to uint32_t. I just checked in the upstream kernel uid_t is actually __kernel_uid32_t which is unsigned int

__field(unsigned long, i_ino)
__field(unsigned int, i_nlink)
__field(u64, i_version)
Expand All @@ -91,15 +92,15 @@ DECLARE_EVENT_CLASS(zfs_ace_class,
__entry->z_mapcnt = zn->z_mapcnt;
__entry->z_size = zn->z_size;
__entry->z_pflags = zn->z_pflags;
__entry->z_uid = zn->z_uid;
__entry->z_gid = zn->z_gid;
__entry->z_sync_cnt = zn->z_sync_cnt;
__entry->z_mode = zn->z_mode;
__entry->z_is_sa = zn->z_is_sa;
__entry->z_is_mapped = zn->z_is_mapped;
__entry->z_is_ctldir = zn->z_is_ctldir;
__entry->z_is_stale = zn->z_is_stale;

__entry->i_uid = zfs_uid_read(ZTOI(zn));
__entry->i_gid = zfs_gid_read(ZTOI(zn));
__entry->i_ino = zn->z_inode.i_ino;
__entry->i_nlink = zn->z_inode.i_nlink;
__entry->i_version = zn->z_inode.i_version;
Expand All @@ -118,22 +119,22 @@ DECLARE_EVENT_CLASS(zfs_ace_class,
TP_printk("zn { id %llu unlinked %u atime_dirty %u "
"zn_prefetch %u moved %u blksz %u seq %u "
"mapcnt %llu size %llu pflags %llu "
"uid %llu gid %llu sync_cnt %u mode 0x%x is_sa %d "
"sync_cnt %u mode 0x%x is_sa %d "
"is_mapped %d is_ctldir %d is_stale %d inode { "
"ino %lu nlink %u version %llu size %lli blkbits %u "
"bytes %u mode 0x%x generation %x } } ace { type %u "
"flags %u access_mask %u } mask_matched %u",
"uid %u gid %u ino %lu nlink %u version %llu size %lli "
"blkbits %u bytes %u mode 0x%x generation %x } } "
"ace { type %u flags %u access_mask %u } mask_matched %u",
__entry->z_id, __entry->z_unlinked, __entry->z_atime_dirty,
__entry->z_zn_prefetch, __entry->z_moved, __entry->z_blksz,
__entry->z_seq, __entry->z_mapcnt, __entry->z_size,
__entry->z_pflags, __entry->z_uid,
__entry->z_gid, __entry->z_sync_cnt, __entry->z_mode,
__entry->z_pflags, __entry->z_sync_cnt, __entry->z_mode,
__entry->z_is_sa, __entry->z_is_mapped,
__entry->z_is_ctldir, __entry->z_is_stale, __entry->i_ino,
__entry->i_nlink, __entry->i_version, __entry->i_size,
__entry->i_blkbits, __entry->i_bytes, __entry->i_mode,
__entry->i_generation, __entry->z_type, __entry->z_flags,
__entry->z_access_mask, __entry->mask_matched)
__entry->z_is_ctldir, __entry->z_is_stale, __entry->i_uid,
__entry->i_gid, __entry->i_ino, __entry->i_nlink,
__entry->i_version, __entry->i_size, __entry->i_blkbits,
__entry->i_bytes, __entry->i_mode, __entry->i_generation,
__entry->z_type, __entry->z_flags, __entry->z_access_mask,
__entry->mask_matched)
);

#define DEFINE_ACE_EVENT(name) \
Expand Down
2 changes: 0 additions & 2 deletions include/sys/zfs_znode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ typedef struct znode {
uint64_t z_dnodesize; /* dnode size */
uint64_t z_size; /* file size (cached) */
uint64_t z_pflags; /* pflags (cached) */
uint64_t z_uid; /* uid fuid (cached) */
uint64_t z_gid; /* gid fuid (cached) */
uint32_t z_sync_cnt; /* synchronous open count */
mode_t z_mode; /* mode (cached) */
kmutex_t z_acl_lock; /* acl data lock */
Expand Down
27 changes: 17 additions & 10 deletions module/zfs/zfs_acl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include <sys/zap.h>
#include <sys/sa.h>
#include <sys/trace_acl.h>
#include <sys/zpl.h>
#include "fs/fs_subr.h"

#define ALLOW ACE_ACCESS_ALLOWED_ACE_TYPE
Expand Down Expand Up @@ -1166,7 +1167,8 @@ zfs_acl_chown_setattr(znode_t *zp)
error = zfs_acl_node_read(zp, B_TRUE, &aclp, B_FALSE);
if (error == 0 && aclp->z_acl_count > 0)
zp->z_mode = zfs_mode_compute(zp->z_mode, aclp,
&zp->z_pflags, zp->z_uid, zp->z_gid);
&zp->z_pflags, KUID_TO_SUID(ZTOI(zp)->i_uid),
KGID_TO_SGID(ZTOI(zp)->i_gid));

/*
* Some ZFS implementations (ZEVO) create neither a ZNODE_ACL
Expand Down Expand Up @@ -1324,7 +1326,7 @@ zfs_aclset_common(znode_t *zp, zfs_acl_t *aclp, cred_t *cr, dmu_tx_t *tx)
mode = zp->z_mode;

mode = zfs_mode_compute(mode, aclp, &zp->z_pflags,
zp->z_uid, zp->z_gid);
zfs_uid_read(ZTOI(zp)), zfs_gid_read(ZTOI(zp)));

zp->z_mode = mode;
SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_MODE(zsb), NULL,
Expand Down Expand Up @@ -1778,7 +1780,7 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr,
(uint64_t)vap->va_gid,
cr, ZFS_GROUP, &acl_ids->z_fuidp);
gid = vap->va_gid;
if (acl_ids->z_fgid != dzp->z_gid &&
if (acl_ids->z_fgid != KGID_TO_SGID(ZTOI(dzp)->i_gid) &&
!groupmember(vap->va_gid, cr) &&
secpolicy_vnode_create_gid(cr) != 0)
acl_ids->z_fgid = 0;
Expand All @@ -1788,7 +1790,8 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr,
char *domain;
uint32_t rid;

acl_ids->z_fgid = dzp->z_gid;
acl_ids->z_fgid = KGID_TO_SGID(
ZTOI(dzp)->i_gid);
gid = zfs_fuid_map_id(zsb, acl_ids->z_fgid,
cr, ZFS_GROUP);

Expand Down Expand Up @@ -2340,7 +2343,8 @@ zfs_has_access(znode_t *zp, cred_t *cr)
if (zfs_zaccess_aces_check(zp, &have, B_TRUE, cr) != 0) {
uid_t owner;

owner = zfs_fuid_map_id(ZTOZSB(zp), zp->z_uid, cr, ZFS_OWNER);
owner = zfs_fuid_map_id(ZTOZSB(zp),
KUID_TO_SUID(ZTOI(zp)->i_uid), cr, ZFS_OWNER);
return (secpolicy_vnode_any_access(cr, ZTOI(zp), owner) == 0);
}
return (B_TRUE);
Expand Down Expand Up @@ -2418,12 +2422,13 @@ zfs_fastaccesschk_execute(znode_t *zdp, cred_t *cr)
return (0);
}

if (FUID_INDEX(zdp->z_uid) != 0 || FUID_INDEX(zdp->z_gid) != 0) {
if (KUID_TO_SUID(ZTOI(zdp)->i_uid) != 0 ||
KGID_TO_SGID(ZTOI(zdp)->i_gid) != 0) {
mutex_exit(&zdp->z_acl_lock);
goto slow;
}

if (uid == zdp->z_uid) {
if (uid == KUID_TO_SUID(ZTOI(zdp)->i_uid)) {
owner = B_TRUE;
if (zdp->z_mode & S_IXUSR) {
mutex_exit(&zdp->z_acl_lock);
Expand All @@ -2433,7 +2438,7 @@ zfs_fastaccesschk_execute(znode_t *zdp, cred_t *cr)
goto slow;
}
}
if (groupmember(zdp->z_gid, cr)) {
if (groupmember(KGID_TO_SGID(ZTOI(zdp)->i_gid), cr)) {
groupmbr = B_TRUE;
if (zdp->z_mode & S_IXGRP) {
mutex_exit(&zdp->z_acl_lock);
Expand Down Expand Up @@ -2513,7 +2518,8 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
}
}

owner = zfs_fuid_map_id(ZTOZSB(zp), zp->z_uid, cr, ZFS_OWNER);
owner = zfs_fuid_map_id(ZTOZSB(zp), KUID_TO_SUID(ZTOI(zp)->i_uid),
cr, ZFS_OWNER);
/*
* Map the bits required to the standard inode flags
* S_IRUSR|S_IWUSR|S_IXUSR in the needed_bits. Map the bits
Expand Down Expand Up @@ -2642,7 +2648,8 @@ zfs_delete_final_check(znode_t *zp, znode_t *dzp,
int error;
uid_t downer;

downer = zfs_fuid_map_id(ZTOZSB(dzp), dzp->z_uid, cr, ZFS_OWNER);
downer = zfs_fuid_map_id(ZTOZSB(dzp), KUID_TO_SUID(ZTOI(dzp)->i_uid),
cr, ZFS_OWNER);

error = secpolicy_vnode_access2(cr, ZTOI(dzp),
downer, available_perms, S_IWUSR|S_IXUSR);
Expand Down
2 changes: 0 additions & 2 deletions module/zfs/zfs_ctldir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,6 @@ zfsctl_inode_alloc(zfs_sb_t *zsb, uint64_t id,
zp->z_mapcnt = 0;
zp->z_size = 0;
zp->z_pflags = 0;
zp->z_uid = 0;
zp->z_gid = 0;
zp->z_mode = 0;
zp->z_sync_cnt = 0;
zp->z_is_mapped = B_FALSE;
Expand Down
6 changes: 4 additions & 2 deletions module/zfs/zfs_dir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1104,8 +1104,10 @@ zfs_sticky_remove_access(znode_t *zdp, znode_t *zp, cred_t *cr)
if ((zdp->z_mode & S_ISVTX) == 0)
return (0);

downer = zfs_fuid_map_id(zsb, zdp->z_uid, cr, ZFS_OWNER);
fowner = zfs_fuid_map_id(zsb, zp->z_uid, cr, ZFS_OWNER);
downer = zfs_fuid_map_id(zsb, KUID_TO_SUID(ZTOI(zdp)->i_uid),
cr, ZFS_OWNER);
fowner = zfs_fuid_map_id(zsb, KUID_TO_SUID(ZTOI(zp)->i_uid),
cr, ZFS_OWNER);

if ((uid = crgetuid(cr)) == downer || uid == fowner ||
(S_ISDIR(ZTOI(zp)->i_mode) &&
Expand Down
6 changes: 4 additions & 2 deletions module/zfs/zfs_fuid.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,10 @@ zfs_fuid_find_by_idx(zfs_sb_t *zsb, uint32_t idx)
void
zfs_fuid_map_ids(znode_t *zp, cred_t *cr, uid_t *uidp, uid_t *gidp)
{
*uidp = zfs_fuid_map_id(ZTOZSB(zp), zp->z_uid, cr, ZFS_OWNER);
*gidp = zfs_fuid_map_id(ZTOZSB(zp), zp->z_gid, cr, ZFS_GROUP);
*uidp = zfs_fuid_map_id(ZTOZSB(zp), KUID_TO_SUID(ZTOI(zp)->i_uid),
cr, ZFS_OWNER);
*gidp = zfs_fuid_map_id(ZTOZSB(zp), KGID_TO_SGID(ZTOI(zp)->i_gid),
cr, ZFS_GROUP);
}

uid_t
Expand Down
12 changes: 6 additions & 6 deletions module/zfs/zfs_log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,13 @@ zfs_log_create(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
/* Store dnode slot count in 8 bits above object id. */
LR_FOID_SET_SLOTS(lr->lr_foid, zp->z_dnodesize >> DNODE_SHIFT);
lr->lr_mode = zp->z_mode;
if (!IS_EPHEMERAL(zp->z_uid)) {
lr->lr_uid = (uint64_t)zp->z_uid;
if (!IS_EPHEMERAL(KUID_TO_SUID(ZTOI(zp)->i_uid))) {
lr->lr_uid = (uint64_t)KUID_TO_SUID(ZTOI(zp)->i_uid);
} else {
lr->lr_uid = fuidp->z_fuid_owner;
}
if (!IS_EPHEMERAL(zp->z_gid)) {
lr->lr_gid = (uint64_t)zp->z_gid;
if (!IS_EPHEMERAL(KGID_TO_SGID(ZTOI(zp)->i_gid))) {
lr->lr_gid = (uint64_t)KGID_TO_SGID(ZTOI(zp)->i_gid);
} else {
lr->lr_gid = fuidp->z_fuid_group;
}
Expand Down Expand Up @@ -407,8 +407,8 @@ zfs_log_symlink(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
lr = (lr_create_t *)&itx->itx_lr;
lr->lr_doid = dzp->z_id;
lr->lr_foid = zp->z_id;
lr->lr_uid = zp->z_uid;
lr->lr_gid = zp->z_gid;
lr->lr_uid = KUID_TO_SUID(ZTOI(zp)->i_uid);
lr->lr_gid = KGID_TO_SGID(ZTOI(zp)->i_gid);
lr->lr_mode = zp->z_mode;
(void) sa_lookup(zp->z_sa_hdl, SA_ZPL_GEN(ZTOZSB(zp)), &lr->lr_gen,
sizeof (uint64_t));
Expand Down
3 changes: 2 additions & 1 deletion module/zfs/zfs_vfsops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,10 +627,11 @@ zfs_owner_overquota(zfs_sb_t *zsb, znode_t *zp, boolean_t isgroup)
{
uint64_t fuid;
uint64_t quotaobj;
struct inode *ip = ZTOI(zp);

quotaobj = isgroup ? zsb->z_groupquota_obj : zsb->z_userquota_obj;

fuid = isgroup ? zp->z_gid : zp->z_uid;
fuid = isgroup ? KGID_TO_SGID(ip->i_gid) : KUID_TO_SUID(ip->i_uid);

if (quotaobj == 0 || zsb->z_replay)
return (B_FALSE);
Expand Down
Loading