Date: Tue, 14 Jul 2009 17:21:00 GMT From: Edward Tomasz Napierala <trasz@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 166086 for review Message-ID: <200907141721.n6EHL0Pl021251@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=166086 Change 166086 by trasz@trasz_victim on 2009/07/14 17:20:55 Beat the code into shape in order to make it compile, boot, and pass the 'acltools' regression tests. Affected files ... .. //depot/projects/soc2008/trasz_nfs4acl/TODO#55 edit .. //depot/projects/soc2008/trasz_nfs4acl/bin/chmod/chmod.c#6 edit .. //depot/projects/soc2008/trasz_nfs4acl/bin/cp/utils.c#10 edit .. //depot/projects/soc2008/trasz_nfs4acl/bin/getfacl/getfacl.c#13 edit .. //depot/projects/soc2008/trasz_nfs4acl/bin/ls/print.c#5 edit .. //depot/projects/soc2008/trasz_nfs4acl/bin/mv/mv.c#7 edit .. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.c#16 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c#29 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_posix1e.c#15 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_subr.c#38 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/sys/vnode.h#33 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#32 edit .. //depot/projects/soc2008/trasz_nfs4acl/tools/regression/acltools/tools-posix.test#9 edit Differences ... ==== //depot/projects/soc2008/trasz_nfs4acl/TODO#55 (text+ko) ==== @@ -1,6 +1,6 @@ Things that need to be done before this goes into -CURRENT: -- Don't use fpathconf(..., _PC_EXTENDED_SECURITY_NP); instead just call +- Don't use fpathconf(..., _PC_ACL_NFS4); instead just call acl_set_file(3) or acl_get_file(3) with ACL_TYPE_NFS4 and handle EOPNOTSUPP. - Decide how VAPPEND is supposed to work - always OR-ed with VWRITE, ==== //depot/projects/soc2008/trasz_nfs4acl/bin/chmod/chmod.c#6 (text+ko) ==== @@ -42,6 +42,7 @@ __FBSDID("$FreeBSD: src/bin/chmod/chmod.c,v 1.35 2009/07/01 15:52:19 trasz Exp $"); #include <sys/types.h> +#include <sys/param.h> #include <sys/stat.h> #include <err.h> @@ -54,7 +55,7 @@ #include <unistd.h> static void usage(void); -static int may_have_nfs4acl(const FTSENT *ent); +static int may_have_nfs4acl(const FTSENT *ent, int hflag); int main(int argc, char *argv[]) @@ -62,11 +63,10 @@ FTS *ftsp; FTSENT *p; mode_t *set; - int Hflag, Lflag, Rflag, ch, fflag, fts_options, hflag, rval; + int Hflag, Lflag, Rflag, ch, fflag, fts_options, hflag, rval, error; int vflag; char *mode; mode_t newmode; - int (*change_mode)(const char *, mode_t); set = NULL; Hflag = Lflag = Rflag = fflag = hflag = vflag = 0; @@ -140,11 +140,6 @@ } else fts_options = hflag ? FTS_PHYSICAL : FTS_LOGICAL; - if (hflag) - change_mode = lchmod; - else - change_mode = chmod; - mode = *argv; if ((set = setmode(mode)) == NULL) errx(1, "invalid file mode: %s", mode); @@ -186,17 +181,14 @@ * identical to the one computed from an ACL will change * that ACL. */ ->>>> ORIGINAL //depot/vendor/freebsd/src/bin/chmod/chmod.c#13 -==== THEIRS //depot/vendor/freebsd/src/bin/chmod/chmod.c#14 - if (may_have_nfs4acl(p) == 0 && + if (may_have_nfs4acl(p, hflag) == 0 && (newmode & ALLPERMS) == (p->fts_statp->st_mode & ALLPERMS)) -==== YOURS //trasz_victim/nfs4acl/bin/chmod/chmod.c - if (may_have_nfs4acl(p) == 0) { - if ((newmode & ALLPERMS) == (p->fts_statp->st_mode & ALLPERMS)) -<<<< continue; - } - if ((*change_mode)(p->fts_accpath, newmode) && !fflag) { + if (hflag) + error = lchmod(p->fts_accpath, newmode); + else + error = chmod(p->fts_accpath, newmode); + if (error && !fflag) { warn("%s", p->fts_path); rval = 1; } else { @@ -235,22 +227,20 @@ } static int -may_have_nfs4acl(const FTSENT *ent) +may_have_nfs4acl(const FTSENT *ent, int hflag) { int ret; - static dev_t previous_dev = (dev_t)-1; + static dev_t previous_dev = NODEV; static int supports_acls = -1; if (previous_dev != ent->fts_statp->st_dev) { previous_dev = ent->fts_statp->st_dev; supports_acls = 0; ->>>> ORIGINAL //depot/vendor/freebsd/src/bin/chmod/chmod.c#13 -==== THEIRS //depot/vendor/freebsd/src/bin/chmod/chmod.c#14 - ret = pathconf(ent->fts_accpath, _PC_ACL_NFS4); -==== YOURS //trasz_victim/nfs4acl/bin/chmod/chmod.c - ret = pathconf(ent->fts_accpath, _PC_EXTENDED_SECURITY_NP); -<<<< + if (hflag) + ret = lpathconf(ent->fts_accpath, _PC_ACL_NFS4); + else + ret = pathconf(ent->fts_accpath, _PC_ACL_NFS4); if (ret > 0) supports_acls = 1; else if (ret < 0 && errno != EINVAL) ==== //depot/projects/soc2008/trasz_nfs4acl/bin/cp/utils.c#10 (text+ko) ==== @@ -386,7 +386,7 @@ source_type = ACL_TYPE_ACCESS; } - if (fpathconf(source_fd, _PC_EXTENDED_SECURITY_NP) == 1) { + if (fpathconf(source_fd, _PC_ACL_NFS4) == 1) { source_acl_supported = 1; source_type = ACL_TYPE_NFS4; } @@ -396,7 +396,7 @@ dest_type = ACL_TYPE_ACCESS; } - if (fpathconf(dest_fd, _PC_EXTENDED_SECURITY_NP) == 1) { + if (fpathconf(dest_fd, _PC_ACL_NFS4) == 1) { dest_acl_supported = 1; dest_type = ACL_TYPE_NFS4; } @@ -449,7 +449,7 @@ source_type = ACL_TYPE_ACCESS; } - if (pathconf(source_dir, _PC_EXTENDED_SECURITY_NP) == 1) { + if (pathconf(source_dir, _PC_ACL_NFS4) == 1) { source_acl_supported = 1; source_type = ACL_TYPE_NFS4; } @@ -459,7 +459,7 @@ dest_type = ACL_TYPE_ACCESS; } - if (pathconf(dest_dir, _PC_EXTENDED_SECURITY_NP) == 1) { + if (pathconf(dest_dir, _PC_ACL_NFS4) == 1) { dest_acl_supported = 1; dest_type = ACL_TYPE_NFS4; } ==== //depot/projects/soc2008/trasz_nfs4acl/bin/getfacl/getfacl.c#13 (text+ko) ==== @@ -197,7 +197,7 @@ else more_than_one++; - if (pathconf(path, _PC_EXTENDED_SECURITY_NP)) { + if (pathconf(path, _PC_ACL_NFS4)) { if (type == ACL_TYPE_DEFAULT) { warnx("%s: there are no default entries in NFSv4 ACLs", path); ==== //depot/projects/soc2008/trasz_nfs4acl/bin/ls/print.c#5 (text+ko) ==== @@ -70,7 +70,7 @@ static void endcolor(int); static int colortype(mode_t); #endif -static void aclmode(char *, const FTSENT *, int *); +static void aclmode(char *, const FTSENT *); #define IS_NOPRINT(p) ((p)->fts_number == NO_PRINT) @@ -139,16 +139,12 @@ #ifdef COLORLS int color_printed = 0; #endif - int haveacls; - dev_t prevdev; if ((dp->list == NULL || dp->list->fts_level != FTS_ROOTLEVEL) && (f_longform || f_size)) { (void)printf("total %lu\n", howmany(dp->btotal, blocksize)); } - haveacls = 1; - prevdev = (dev_t)-1; for (p = dp->list; p; p = p->fts_link) { if (IS_NOPRINT(p)) continue; @@ -159,14 +155,7 @@ (void)printf("%*jd ", dp->s_block, howmany(sp->st_blocks, blocksize)); strmode(sp->st_mode, buf); - /* - * Cache whether or not the filesystem supports ACL's to - * avoid expensive syscalls. Try again when we change devices. - */ - if (haveacls || sp->st_dev != prevdev) { - aclmode(buf, p, &haveacls); - prevdev = sp->st_dev; - } + aclmode(buf, p); np = p->fts_pointer; (void)printf("%s %*u %-*s %-*s ", buf, dp->s_nlink, sp->st_nlink, dp->s_user, np->user, dp->s_group, @@ -612,73 +601,68 @@ (void)printf("%*jd ", (u_int)width, bytes); } +/* + * Add a + after the standard rwxrwxrwx mode if the file has an + * ACL. strmode() reserves space at the end of the string. + */ static void -aclmode(char *buf, const FTSENT *p, int *haveacls) +aclmode(char *buf, const FTSENT *p) { char name[MAXPATHLEN + 1]; - int type = ACL_TYPE_ACCESS, ret, trivial; + int ret, trivial; + static dev_t previous_dev = NODEV; + static int supports_acls = -1; + static int type = ACL_TYPE_ACCESS; acl_t facl; /* - * Add a + after the standard rwxrwxrwx mode if the file has an - * extended ACL. strmode() reserves space at the end of the string. + * XXX: ACLs are not supported on whiteouts and device files + * residing on UFS. */ - if (p->fts_level == FTS_ROOTLEVEL) - snprintf(name, sizeof(name), "%s", p->fts_name); - else - snprintf(name, sizeof(name), "%s/%s", - p->fts_parent->fts_accpath, p->fts_name); - /* - * We have no way to tell whether a symbolic link has an ACL since - * pathconf() and acl_get_file() both follow them. They also don't - * support whiteouts. - */ - if (S_ISLNK(p->fts_statp->st_mode) || S_ISWHT(p->fts_statp->st_mode)) { - *haveacls = 1; + if (S_ISCHR(p->fts_statp->st_mode) || S_ISBLK(p->fts_statp->st_mode) || + S_ISWHT(p->fts_statp->st_mode)) return; - } - *haveacls = 0; + if (previous_dev != p->fts_statp->st_dev) { + previous_dev = p->fts_statp->st_dev; + supports_acls = 0; - ret = pathconf(name, _PC_ACL_EXTENDED); - if (ret > 0) { - type = ACL_TYPE_ACCESS; - *haveacls = 1; - } else if (ret < 0 && errno != EINVAL) { + if (p->fts_level == FTS_ROOTLEVEL) + snprintf(name, sizeof(name), "%s", p->fts_name); + else + snprintf(name, sizeof(name), "%s/%s", + p->fts_parent->fts_accpath, p->fts_name); + ret = lpathconf(name, _PC_ACL_NFS4); + if (ret > 0) { + type = ACL_TYPE_NFS4; + supports_acls = 1; + } else if (ret < 0 && errno != EINVAL) { + warn("%s", name); + return; + } + if (supports_acls == 0) { + ret = lpathconf(name, _PC_ACL_EXTENDED); + if (ret > 0) { + type = ACL_TYPE_ACCESS; + supports_acls = 1; + } else if (ret < 0 && errno != EINVAL) { + warn("%s", name); + return; + } + } + } + if (supports_acls == 0) + return; + facl = acl_get_link_np(name, type); + if (facl == NULL) { warn("%s", name); return; } - - ret = pathconf(name, _PC_EXTENDED_SECURITY_NP); - if (ret > 0) { - type = ACL_TYPE_NFS4; - *haveacls = 1; - } else if (ret < 0 && errno != EINVAL) { + if (acl_is_trivial_np(facl, &trivial)) { warn("%s", name); return; } - - if (*haveacls == 0) - return; - - if ((facl = acl_get_file(name, type)) != NULL) { - if (acl_is_trivial_np(facl, &trivial)) { - warn("%s", name); - return; - } - - if (!trivial) - buf[10] = '+'; - - acl_free(facl); - } else { - /* - * We were denied access. So, obviously, there - * is some ACL denying READ_ACL there. - */ - if (errno == EPERM || errno == EACCES) - buf[10] = '+'; - - warn("%s", name); - } + if (!trivial) + buf[10] = '+'; + acl_free(facl); } ==== //depot/projects/soc2008/trasz_nfs4acl/bin/mv/mv.c#7 (text+ko) ==== @@ -447,7 +447,7 @@ source_type = ACL_TYPE_ACCESS; } - if (fpathconf(source_fd, _PC_EXTENDED_SECURITY_NP) == 1) { + if (fpathconf(source_fd, _PC_ACL_NFS4) == 1) { source_acl_supported = 1; source_type = ACL_TYPE_NFS4; } @@ -457,7 +457,7 @@ dest_type = ACL_TYPE_ACCESS; } - if (fpathconf(dest_fd, _PC_EXTENDED_SECURITY_NP) == 1) { + if (fpathconf(dest_fd, _PC_ACL_NFS4) == 1) { dest_acl_supported = 1; dest_type = ACL_TYPE_NFS4; } ==== //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.c#16 (text+ko) ==== @@ -244,7 +244,7 @@ TAILQ_FOREACH(file, &filelist, next) { local_error = 0; - if (pathconf(file->filename, _PC_EXTENDED_SECURITY_NP)) { + if (pathconf(file->filename, _PC_ACL_NFS4)) { if (acl_type == ACL_TYPE_DEFAULT) { warnx("%s: there are no default entries " "in NFSv4 ACLs", file->filename); ==== //depot/projects/soc2008/trasz_nfs4acl/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c#29 (text+ko) ==== @@ -3834,17 +3834,21 @@ #endif case _PC_ACL_EXTENDED: - *valp = 0; /* TODO */ + *valp = 0; + return (0); + + case _PC_ACL_NFS4: + *valp = 1; + return (0); + + case _PC_ACL_PATH_MAX: + *valp = ACL_MAX_ENTRIES; return (0); case _PC_MIN_HOLE_SIZE: *valp = (int)SPA_MINBLOCKSIZE; return (0); - case _PC_EXTENDED_SECURITY_NP: - *valp = 1; - return (0); - default: return (EOPNOTSUPP); } @@ -4426,6 +4430,26 @@ return (error); } +static int +zfs_freebsd_fifo_pathconf(ap) + struct vop_pathconf_args /* { + struct vnode *a_vp; + int a_name; + register_t *a_retval; + } */ *ap; +{ + + switch (ap->a_name) { + case _PC_ACL_EXTENDED: + case _PC_ACL_NFS4: + case _PC_ACL_PATH_MAX: + case _PC_MAC_PRESENT: + return (zfs_freebsd_pathconf(ap)); + default: + return (fifo_specops.vop_pathconf(ap)); + } +} + /* * FreeBSD's extended attributes namespace defines file name prefix for ZFS' * extended attribute name: @@ -4914,6 +4938,7 @@ .vop_reclaim = zfs_freebsd_reclaim, .vop_setattr = zfs_freebsd_setattr, .vop_write = VOP_PANIC, + .vop_pathconf = zfs_freebsd_fifo_pathconf, .vop_fid = zfs_freebsd_fid, .vop_getacl = zfs_freebsd_getacl, .vop_setacl = zfs_freebsd_setacl, ==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_posix1e.c#15 (text+ko) ==== @@ -59,7 +59,7 @@ accmode_t dac_granted; accmode_t priv_granted; accmode_t acl_mask_granted; - int group_matched, i; + int group_matched, i, error; /* * Look for a normal, non-privileged way to access the file/directory @@ -71,6 +71,10 @@ if (privused != NULL) *privused = 0; + error = vfs_unixify_accmode(&accmode); + if (error) + return (error); + /* * Determine privileges now, but don't apply until we've found a DAC * entry that matches but has failed to allow access. ==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_subr.c#38 (text+ko) ==== @@ -3519,6 +3519,7 @@ { accmode_t dac_granted; accmode_t priv_granted; + int error; /* * Look for a normal, non-privileged way to access the file/directory @@ -3528,6 +3529,10 @@ if (privused != NULL) *privused = 0; + error = vfs_unixify_accmode(&accmode); + if (error) + return (error); + dac_granted = 0; /* Check the owner. */ ==== //depot/projects/soc2008/trasz_nfs4acl/sys/sys/vnode.h#33 (text+ko) ==== @@ -617,6 +617,9 @@ int vaccess_acl_posix1e(enum vtype type, uid_t file_uid, gid_t file_gid, struct acl *acl, accmode_t accmode, struct ucred *cred, int *privused); +int vaccess_acl_nfs4(enum vtype type, uid_t file_uid, gid_t file_gid, + struct acl *aclp, accmode_t accmode, struct ucred *cred, + int *privused); void vattr_null(struct vattr *vap); int vcount(struct vnode *vp); void vdrop(struct vnode *); ==== //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#32 (text+ko) ==== @@ -88,6 +88,7 @@ #include <ufs/ffs/ffs_extern.h> +static vop_access_t ufs_access; static vop_accessx_t ufs_accessx; static int ufs_chmod(struct vnode *, int, struct ucred *, struct thread *); static int ufs_chown(struct vnode *, uid_t, gid_t, struct ucred *, struct thread *); @@ -298,6 +299,19 @@ } static int +ufs_access(ap) + struct vop_access_args /* { + struct vnode *a_vp; + accmode_t a_accmode; + struct ucred *a_cred; + struct thread *a_td; + } */ *ap; +{ + + return (VOP_ACCESSX(ap->a_vp, ap->a_accmode, ap->a_cred, ap->a_td)); +} + +static int ufs_accessx(ap) struct vop_accessx_args /* { struct vnode *a_vp; @@ -308,7 +322,6 @@ { struct vnode *vp = ap->a_vp; struct inode *ip = VTOI(vp); - accmode_t accmode = ap->a_accmode; int error; #ifdef QUOTA int relocked; @@ -323,7 +336,7 @@ * unless the file is a socket, fifo, or a block or * character device resident on the filesystem. */ - if (accmode & VMODIFY_PERMS) { + if (ap->a_accmode & VMODIFY_PERMS) { switch (vp->v_type) { case VDIR: case VLNK: @@ -373,7 +386,7 @@ * "& ~VADMIN_PERMS" is here, because without it, * it would be impossible to remove the IMMUTABLE flag. */ - if ((accmode & (VMODIFY_PERMS & ~VADMIN_PERMS)) && + if ((ap->a_accmode & (VMODIFY_PERMS & ~VADMIN_PERMS)) && (ip->i_flags & (IMMUTABLE | SF_SNAPSHOT))) return (EPERM); @@ -2220,6 +2233,7 @@ switch (ap->a_name) { case _PC_ACL_EXTENDED: + case _PC_ACL_NFS4: case _PC_ACL_PATH_MAX: case _PC_MAC_PRESENT: return (ufs_pathconf(ap)); @@ -2273,7 +2287,7 @@ #endif break; - case _PC_EXTENDED_SECURITY_NP: + case _PC_ACL_NFS4: #ifdef UFS_ACL if (ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) *ap->a_retval = 1; @@ -2286,7 +2300,7 @@ case _PC_ACL_PATH_MAX: #ifdef UFS_ACL - if (ap->a_vp->v_mount->mnt_flag & MNT_ACLS) + if (ap->a_vp->v_mount->mnt_flag & (MNT_ACLS | MNT_NFS4ACLS)) *ap->a_retval = ACL_MAX_ENTRIES; else *ap->a_retval = 3; @@ -2618,6 +2632,7 @@ .vop_read = VOP_PANIC, .vop_reallocblks = VOP_PANIC, .vop_write = VOP_PANIC, + .vop_access = ufs_access, .vop_accessx = ufs_accessx, .vop_bmap = ufs_bmap, .vop_cachedlookup = ufs_lookup, @@ -2662,6 +2677,7 @@ struct vop_vector ufs_fifoops = { .vop_default = &fifo_specops, .vop_fsync = VOP_PANIC, + .vop_access = ufs_access, .vop_accessx = ufs_accessx, .vop_close = ufsfifo_close, .vop_getattr = ufs_getattr, ==== //depot/projects/soc2008/trasz_nfs4acl/tools/regression/acltools/tools-posix.test#9 (text+ko) ==== @@ -77,9 +77,8 @@ > mask::rwx > other::r-x -# XXX: Why doesn't ls(1) print '+' for symbolic links with ACL set? $ ls -l lll | cut -d' ' -f1 -> lrwxrwxr-x +> lrwxrwxr-x+ # Check whether the original file is left untouched. $ ls -l xxx | cut -d' ' -f1 @@ -411,3 +410,20 @@ $ rm fff +# Test if we deal properly with device files. +$ mknod bbb b 1 1 +$ setfacl -m u:42:r,g:43:w bbb +> setfacl: bbb: acl_get_file() failed: Operation not supported +$ ls -l bbb | cut -d' ' -f1 +> brw-r--r-- + +$ rm bbb + +$ mknod ccc c 1 1 +$ setfacl -m u:42:r,g:43:w ccc +> setfacl: ccc: acl_get_file() failed: Operation not supported +$ ls -l ccc | cut -d' ' -f1 +> crw-r--r-- + +$ rm ccc +
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200907141721.n6EHL0Pl021251>