From 386808d4a03ba28c776f26054ab8c62db02aba1f Mon Sep 17 00:00:00 2001 From: elad Date: Mon, 20 Apr 2009 18:06:26 +0000 Subject: [PATCH] Refactor some duplicated file-system code. Proposed and received no objections on tech-kern@: http://mail-index.netbsd.org/tech-kern/2009/04/18/msg004843.html --- sys/fs/ptyfs/ptyfs_vnops.c | 33 +++----- sys/fs/tmpfs/tmpfs_subr.c | 40 +++------ sys/fs/udf/udf_vnops.c | 55 +++---------- sys/kern/vfs_subr.c | 148 +++++++++++++++++++++++++++++++++- sys/sys/vnode.h | 6 +- sys/ufs/ext2fs/ext2fs_vnops.c | 35 +++----- sys/ufs/ufs/ufs_vnops.c | 37 +++------ 7 files changed, 210 insertions(+), 144 deletions(-) diff --git a/sys/fs/ptyfs/ptyfs_vnops.c b/sys/fs/ptyfs/ptyfs_vnops.c index d700fcf3f3e0..4adadb409f88 100644 --- a/sys/fs/ptyfs/ptyfs_vnops.c +++ b/sys/fs/ptyfs/ptyfs_vnops.c @@ -1,4 +1,4 @@ -/* $NetBSD: ptyfs_vnops.c,v 1.27 2008/01/02 11:48:43 ad Exp $ */ +/* $NetBSD: ptyfs_vnops.c,v 1.28 2009/04/20 18:06:27 elad Exp $ */ /* * Copyright (c) 1993, 1995 @@ -76,7 +76,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ptyfs_vnops.c,v 1.27 2008/01/02 11:48:43 ad Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ptyfs_vnops.c,v 1.28 2009/04/20 18:06:27 elad Exp $"); #include #include @@ -464,10 +464,11 @@ ptyfs_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred, struct lwp *l) struct ptyfsnode *ptyfs = VTOPTYFS(vp); int error; - if (kauth_cred_geteuid(cred) != ptyfs->ptyfs_uid && - (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, - NULL)) != 0) - return error; + error = common_chmod_allowed(cred, vp, ptyfs->ptyfs_uid, + ptyfs->ptyfs_gid, mode); + if (error) + return (error); + ptyfs->ptyfs_mode &= ~ALLPERMS; ptyfs->ptyfs_mode |= (mode & ALLPERMS); return 0; @@ -482,25 +483,17 @@ ptyfs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred, struct lwp *l) { struct ptyfsnode *ptyfs = VTOPTYFS(vp); - int error, ismember = 0; + int error; if (uid == (uid_t)VNOVAL) uid = ptyfs->ptyfs_uid; if (gid == (gid_t)VNOVAL) gid = ptyfs->ptyfs_gid; - /* - * If we don't own the file, are trying to change the owner - * of the file, or are not a member of the target group, - * the caller's credentials must imply super-user privilege - * or the call fails. - */ - if ((kauth_cred_geteuid(cred) != ptyfs->ptyfs_uid || uid != ptyfs->ptyfs_uid || - (gid != ptyfs->ptyfs_gid && - !(kauth_cred_getegid(cred) == gid || - (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 && ismember)))) && - ((error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, - NULL)) != 0)) - return error; + + error = common_chown_allowed(cred, ptyfs->ptyfs_uid, ptyfs->ptyfs_gid, + uid, gid); + if (error) + return (error); ptyfs->ptyfs_gid = gid; ptyfs->ptyfs_uid = uid; diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c index 52997ce656c9..d2ef3d88efad 100644 --- a/sys/fs/tmpfs/tmpfs_subr.c +++ b/sys/fs/tmpfs/tmpfs_subr.c @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_subr.c,v 1.50 2009/04/11 11:59:04 markd Exp $ */ +/* $NetBSD: tmpfs_subr.c,v 1.51 2009/04/20 18:06:27 elad Exp $ */ /* * Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc. @@ -35,7 +35,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.50 2009/04/11 11:59:04 markd Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.51 2009/04/20 18:06:27 elad Exp $"); #include #include @@ -1018,7 +1018,7 @@ tmpfs_chflags(struct vnode *vp, int flags, kauth_cred_t cred, struct lwp *l) int tmpfs_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred, struct lwp *l) { - int error, ismember = 0; + int error; struct tmpfs_node *node; KASSERT(VOP_ISLOCKED(vp)); @@ -1033,21 +1033,10 @@ tmpfs_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred, struct lwp *l) if (node->tn_flags & (IMMUTABLE | APPEND)) return EPERM; - /* XXX: The following comes from UFS code, and can be found in - * several other file systems. Shouldn't this be centralized - * somewhere? */ - if (kauth_cred_geteuid(cred) != node->tn_uid && - (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, - NULL))) - return error; - if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL) != 0) { - if (vp->v_type != VDIR && (mode & S_ISTXT)) - return EFTYPE; - - if ((kauth_cred_ismember_gid(cred, node->tn_gid, - &ismember) != 0 || !ismember) && (mode & S_ISGID)) - return EPERM; - } + error = common_chmod_allowed(cred, vp, node->tn_uid, node->tn_gid, + mode); + if (error) + return (error); node->tn_mode = (mode & ALLPERMS); @@ -1072,7 +1061,7 @@ int tmpfs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred, struct lwp *l) { - int error, ismember = 0; + int error; struct tmpfs_node *node; KASSERT(VOP_ISLOCKED(vp)); @@ -1095,15 +1084,10 @@ tmpfs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred, if (node->tn_flags & (IMMUTABLE | APPEND)) return EPERM; - /* XXX: The following comes from UFS code, and can be found in - * several other file systems. Shouldn't this be centralized - * somewhere? */ - if ((kauth_cred_geteuid(cred) != node->tn_uid || uid != node->tn_uid || - (gid != node->tn_gid && !(kauth_cred_getegid(cred) == gid || - (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 && ismember)))) && - ((error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, - NULL)) != 0)) - return error; + error = common_chown_allowed(cred, node->tn_uid, node->tn_gid, uid, + gid); + if (error) + return (error); node->tn_uid = uid; node->tn_gid = gid; diff --git a/sys/fs/udf/udf_vnops.c b/sys/fs/udf/udf_vnops.c index 56e031f62c50..de3a70216aed 100644 --- a/sys/fs/udf/udf_vnops.c +++ b/sys/fs/udf/udf_vnops.c @@ -1,4 +1,4 @@ -/* $NetBSD: udf_vnops.c,v 1.38 2009/03/20 23:06:52 reinoud Exp $ */ +/* $NetBSD: udf_vnops.c,v 1.39 2009/04/20 18:06:26 elad Exp $ */ /* * Copyright (c) 2006, 2008 Reinoud Zandijk @@ -32,7 +32,7 @@ #include #ifndef lint -__KERNEL_RCSID(0, "$NetBSD: udf_vnops.c,v 1.38 2009/03/20 23:06:52 reinoud Exp $"); +__KERNEL_RCSID(0, "$NetBSD: udf_vnops.c,v 1.39 2009/04/20 18:06:26 elad Exp $"); #endif /* not lint */ @@ -935,9 +935,8 @@ udf_chown(struct vnode *vp, uid_t new_uid, gid_t new_gid, kauth_cred_t cred) { struct udf_node *udf_node = VTOI(vp); - uid_t euid, uid; - gid_t egid, gid; - int issuperuser, ismember; + uid_t uid; + gid_t gid; int error; #ifdef notyet @@ -965,26 +964,10 @@ udf_chown(struct vnode *vp, uid_t new_uid, gid_t new_gid, if ((gid_t) ((uint32_t) gid) != gid) return EINVAL; - /* - * If we don't own the file, are trying to change the owner of the - * file, or are not a member of the target group, the caller's - * credentials must imply super-user privilege or the call fails. - */ - /* check permissions */ - euid = kauth_cred_geteuid(cred); - egid = kauth_cred_getegid(cred); - if ((error = kauth_cred_ismember_gid(cred, new_gid, &ismember))) - return error; - error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL); - issuperuser = (error == 0); - - if (!issuperuser) { - if ((new_uid != uid) || (euid != uid)) - return EPERM; - if ((new_gid != gid) && !(egid == new_gid || ismember)) - return EPERM; - } + error = common_chown_allowed(cred, uid, gid, new_uid, new_gid); + if (error) + return (error); /* change the ownership */ udf_setownership(udf_node, new_uid, new_gid); @@ -1000,9 +983,8 @@ static int udf_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred) { struct udf_node *udf_node = VTOI(vp); - uid_t euid, uid; - gid_t egid, gid; - int issuperuser, ismember; + uid_t uid; + gid_t gid; int error; #ifdef notyet @@ -1019,22 +1001,9 @@ udf_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred) udf_getownership(udf_node, &uid, &gid); /* check permissions */ - euid = kauth_cred_geteuid(cred); - egid = kauth_cred_getegid(cred); - if ((error = kauth_cred_ismember_gid(cred, gid, &ismember))) - return error; - error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL); - issuperuser = (error == 0); - - if ((euid != uid) && !issuperuser) - return EPERM; - if (euid != 0) { - if (vp->v_type != VDIR && (mode & S_ISTXT)) - return EFTYPE; - - if ((!ismember) && (mode & S_ISGID)) - return EPERM; - } + error = common_chmod_allowed(cred, vp, uid, gid, mode); + if (error) + return (error); /* change mode */ udf_setaccessmode(udf_node, mode); diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c index 1f548b881b88..1eeeae2749cd 100644 --- a/sys/kern/vfs_subr.c +++ b/sys/kern/vfs_subr.c @@ -1,4 +1,4 @@ -/* $NetBSD: vfs_subr.c,v 1.371 2009/04/17 20:22:52 dyoung Exp $ */ +/* $NetBSD: vfs_subr.c,v 1.372 2009/04/20 18:06:26 elad Exp $ */ /*- * Copyright (c) 1997, 1998, 2004, 2005, 2007, 2008 The NetBSD Foundation, Inc. @@ -81,7 +81,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.371 2009/04/17 20:22:52 dyoung Exp $"); +__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.372 2009/04/20 18:06:26 elad Exp $"); #include "opt_ddb.h" #include "opt_compat_netbsd.h" @@ -3206,3 +3206,147 @@ vfs_mount_print(struct mount *mp, int full, void (*pr)(const char *, ...)) } } #endif /* DDB */ + +/* + * Common routine to check if chmod() is allowed. + * + * Policy: + * - You must be root, or + * - You must own the file, and + * - You must not set the "sticky" bit (meaningless, see chmod(2)) + * - You must be a member of the group if you're trying to set the + * SGIDf bit + * + * cred - credentials of the invoker + * vp - vnode of the file-system object + * cur_uid, cur_gid - current uid/gid of the file-system object + * new_mode - new mode for the file-system object + * + * Returns 0 if the change is allowed, or an error value otherwise. + */ +int +common_chmod_allowed(kauth_cred_t cred, struct vnode *vp, uid_t cur_uid, + gid_t cur_gid, mode_t new_mode) +{ + int error; + + /* Superuser can always change mode. */ + error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, + NULL); + if (!error) { + goto out; + } + + /* Otherwise, user must own the file. */ + if (kauth_cred_geteuid(cred) != cur_uid) { + goto out; + } + + /* + * Non-root users can't set the sticky bit on files. + */ + if ((vp->v_type != VDIR) && (new_mode & S_ISTXT)) { + error = EFTYPE; + goto out; + } + + /* + * If the invoker is trying to set the SGID bit on the file, + * check group membership. + */ + if (new_mode & S_ISGID) { + int ismember; + + error = kauth_cred_ismember_gid(cred, cur_gid, + &ismember); + if (error) + goto out; + + if (!ismember) { + error = EPERM; + goto out; + } + } + + error = 0; + + out: + return (error); +} + +/* + * Common routine to check if chown() is allowed. + * + * Policy: + * - You must be root, or + * - You must own the file, and + * - You must not try to change ownership, and + * - You must be member of the new group + * + * cred - credentials of the invoker + * cur_uid, cur_gid - current uid/gid of the file-system object + * new_uid, new_gid - target uid/gid of the file-system object + * + * Returns 0 if the change is allowed, or an error value otherwise. + */ +int +common_chown_allowed(kauth_cred_t cred, uid_t cur_uid, gid_t cur_gid, + uid_t new_uid, gid_t new_gid) +{ + int error, ismember; + + /* + * You can only change ownership of a file if: + * You are the superuser, or... + */ + error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, + NULL); + if (!error) { + goto out; + } + + /* + * You own the file and... + */ + if (kauth_cred_geteuid(cred) == cur_uid) { + /* + * You don't try to change ownership, and... + */ + if (new_uid != cur_uid) { + goto out; + } + + /* + * You don't try to change group (no-op), or... + */ + if (new_gid == cur_gid) { + error = 0; + goto out; + } + + /* + * Your effective gid is the new gid, or... + */ + if (kauth_cred_getegid(cred) == new_gid) { + error = 0; + goto out; + } + + /* + * The new gid is one you're a member of. + */ + ismember = 0; + error = kauth_cred_ismember_gid(cred, new_gid, + &ismember); + if (error || !ismember) { + error = EPERM; + goto out; + } + + error = 0; + } + + out: + return (error); +} + diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h index 09fbf27a2ab1..5c5ba75e7775 100644 --- a/sys/sys/vnode.h +++ b/sys/sys/vnode.h @@ -1,4 +1,4 @@ -/* $NetBSD: vnode.h,v 1.203 2009/03/19 09:07:54 pooka Exp $ */ +/* $NetBSD: vnode.h,v 1.204 2009/04/20 18:06:26 elad Exp $ */ /*- * Copyright (c) 2008 The NetBSD Foundation, Inc. @@ -663,6 +663,10 @@ void vfs_timestamp(struct timespec *); void vfs_vnode_print(struct vnode *, int, void (*)(const char *, ...)); void vfs_mount_print(struct mount *, int, void (*)(const char *, ...)); #endif /* DDB */ + +int common_chmod_allowed(kauth_cred_t, struct vnode *, uid_t, gid_t, + mode_t); +int common_chown_allowed(kauth_cred_t, uid_t, gid_t, uid_t, gid_t); #endif /* _KERNEL */ #endif /* !_SYS_VNODE_H_ */ diff --git a/sys/ufs/ext2fs/ext2fs_vnops.c b/sys/ufs/ext2fs/ext2fs_vnops.c index bcd6fe333ecc..ed0552a321f4 100644 --- a/sys/ufs/ext2fs/ext2fs_vnops.c +++ b/sys/ufs/ext2fs/ext2fs_vnops.c @@ -1,4 +1,4 @@ -/* $NetBSD: ext2fs_vnops.c,v 1.83 2008/11/23 10:09:25 mrg Exp $ */ +/* $NetBSD: ext2fs_vnops.c,v 1.84 2009/04/20 18:06:27 elad Exp $ */ /* * Copyright (c) 1982, 1986, 1989, 1993 @@ -70,7 +70,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ext2fs_vnops.c,v 1.83 2008/11/23 10:09:25 mrg Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ext2fs_vnops.c,v 1.84 2009/04/20 18:06:27 elad Exp $"); #include #include @@ -442,19 +442,12 @@ static int ext2fs_chmod(struct vnode *vp, int mode, kauth_cred_t cred, struct lwp *l) { struct inode *ip = VTOI(vp); - int error, ismember = 0; + int error; - if (kauth_cred_geteuid(cred) != ip->i_uid && - (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, - NULL))) + error = common_chmod_allowed(cred, vp, ip->i_uid, ip->i_gid, mode); + if (error) return (error); - if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL)) { - if (vp->v_type != VDIR && (mode & S_ISTXT)) - return (EFTYPE); - if ((kauth_cred_ismember_gid(cred, ip->i_gid, &ismember) != 0 || - !ismember) && (mode & ISGID)) - return (EPERM); - } + ip->i_e2fs_mode &= ~ALLPERMS; ip->i_e2fs_mode |= (mode & ALLPERMS); ip->i_flag |= IN_CHANGE; @@ -472,23 +465,17 @@ ext2fs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred, struct inode *ip = VTOI(vp); uid_t ouid; gid_t ogid; - int error = 0, ismember = 0; + int error; if (uid == (uid_t)VNOVAL) uid = ip->i_uid; if (gid == (gid_t)VNOVAL) gid = ip->i_gid; - /* - * If we don't own the file, are trying to change the owner - * of the file, or are not a member of the target group, - * the caller must be superuser or the call fails. - */ - if ((kauth_cred_geteuid(cred) != ip->i_uid || uid != ip->i_uid || - (gid != ip->i_gid && - !(kauth_cred_getegid(cred) == gid || - (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 && ismember)))) && - (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL))) + + error = common_chown_allowed(cred, ip->i_uid, ip->i_gid, uid, gid); + if (error) return (error); + ogid = ip->i_gid; ouid = ip->i_uid; diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c index 0669b4c71b3a..8e2c15974466 100644 --- a/sys/ufs/ufs/ufs_vnops.c +++ b/sys/ufs/ufs/ufs_vnops.c @@ -1,4 +1,4 @@ -/* $NetBSD: ufs_vnops.c,v 1.173 2009/02/22 20:28:07 ad Exp $ */ +/* $NetBSD: ufs_vnops.c,v 1.174 2009/04/20 18:06:27 elad Exp $ */ /*- * Copyright (c) 2008 The NetBSD Foundation, Inc. @@ -66,7 +66,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.173 2009/02/22 20:28:07 ad Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.174 2009/04/20 18:06:27 elad Exp $"); #if defined(_KERNEL_OPT) #include "opt_ffs.h" @@ -640,21 +640,16 @@ static int ufs_chmod(struct vnode *vp, int mode, kauth_cred_t cred, struct lwp *l) { struct inode *ip; - int error, ismember = 0; + int error; UFS_WAPBL_JLOCK_ASSERT(vp->v_mount); ip = VTOI(vp); - if (kauth_cred_geteuid(cred) != ip->i_uid && - (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL))) + + error = common_chmod_allowed(cred, vp, ip->i_uid, ip->i_gid, mode); + if (error) return (error); - if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL)) { - if (vp->v_type != VDIR && (mode & S_ISTXT)) - return (EFTYPE); - if ((kauth_cred_ismember_gid(cred, ip->i_gid, &ismember) != 0 || - !ismember) && (mode & ISGID)) - return (EPERM); - } + ip->i_mode &= ~ALLPERMS; ip->i_mode |= (mode & ALLPERMS); ip->i_flag |= IN_CHANGE; @@ -672,7 +667,7 @@ ufs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred, struct lwp *l) { struct inode *ip; - int error, ismember = 0; + int error = 0; #ifdef QUOTA uid_t ouid; gid_t ogid; @@ -685,19 +680,9 @@ ufs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred, uid = ip->i_uid; if (gid == (gid_t)VNOVAL) gid = ip->i_gid; - /* - * If we don't own the file, are trying to change the owner - * of the file, or are not a member of the target group, - * the caller's credentials must imply super-user privilege - * or the call fails. - */ - if ((kauth_cred_geteuid(cred) != ip->i_uid || uid != ip->i_uid || - (gid != ip->i_gid && - !(kauth_cred_getegid(cred) == gid || - (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 && - ismember)))) && - ((error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, - NULL)) != 0)) + + error = common_chown_allowed(cred, ip->i_uid, ip->i_gid, uid, gid); + if (error) return (error); #ifdef QUOTA