From owner-svn-src-all@FreeBSD.ORG Fri Jan 22 11:42:44 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C046D106566B; Fri, 22 Jan 2010 11:42:44 +0000 (UTC) (envelope-from trasz@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id ACEA38FC1F; Fri, 22 Jan 2010 11:42:44 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o0MBgiZQ058123; Fri, 22 Jan 2010 11:42:44 GMT (envelope-from trasz@svn.freebsd.org) Received: (from trasz@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o0MBgi5l058119; Fri, 22 Jan 2010 11:42:44 GMT (envelope-from trasz@svn.freebsd.org) Message-Id: <201001221142.o0MBgi5l058119@svn.freebsd.org> From: Edward Tomasz Napierala Date: Fri, 22 Jan 2010 11:42:44 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r202805 - stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Jan 2010 11:42:44 -0000 Author: trasz Date: Fri Jan 22 11:42:44 2010 New Revision: 202805 URL: http://svn.freebsd.org/changeset/base/202805 Log: MFC c195785: Fix permission handling for extended attributes in ZFS. Without this change, ZFS uses SunOS Alternate Data Streams semantics - each EA has its own permissions, which are set at EA creation time and - unlike SunOS - invisible to the user and impossible to change. From the user point of view, it's just broken: sometimes access is granted when it shouldn't be, sometimes it's denied when it shouldn't be. This patch makes it behave just like UFS, i.e. depend on current file permissions. Also, it fixes returned error codes (ENOATTR instead of ENOENT) and makes listextattr(2) return 0 instead of EPERM where there is no EA directory (i.e. the file never had any EA). Tested by: cperciva Modified: stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Directory Properties: stable/7/sys/ (props changed) stable/7/sys/cddl/contrib/opensolaris/ (props changed) stable/7/sys/contrib/dev/acpica/ (props changed) stable/7/sys/contrib/pf/ (props changed) Modified: stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c ============================================================================== --- stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Fri Jan 22 11:42:23 2010 (r202804) +++ stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Fri Jan 22 11:42:44 2010 (r202805) @@ -2364,6 +2364,15 @@ zfs_zaccess(znode_t *zp, int mode, int f is_attr = ((zp->z_phys->zp_flags & ZFS_XATTR) && (ZTOV(zp)->v_type == VDIR)); +#ifdef __FreeBSD__ + /* + * In FreeBSD, we don't care about permissions of individual ADS. + * Note that not checking them is not just an optimization - without + * this shortcut, EA operations may bogusly fail with EACCES. + */ + if (zp->z_phys->zp_flags & ZFS_XATTR) + return (0); +#else /* * If attribute then validate against base file */ @@ -2389,6 +2398,7 @@ zfs_zaccess(znode_t *zp, int mode, int f mode |= ACE_READ_NAMED_ATTRS; } } +#endif if ((error = zfs_zaccess_common(check_zp, mode, &working_mode, &check_privs, skipaclchk, cr)) == 0) { Modified: stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c ============================================================================== --- stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c Fri Jan 22 11:42:23 2010 (r202804) +++ stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c Fri Jan 22 11:42:44 2010 (r202805) @@ -831,8 +831,14 @@ zfs_make_xattrdir(znode_t *zp, vattr_t * *xvpp = NULL; + /* + * In FreeBSD, access checking for creating an EA is being done + * in zfs_setextattr(), + */ +#ifndef __FreeBSD__ if (error = zfs_zaccess(zp, ACE_WRITE_NAMED_ATTRS, 0, B_FALSE, cr)) return (error); +#endif tx = dmu_tx_create(zfsvfs->z_os); dmu_tx_hold_bonus(tx, zp->z_id); @@ -906,12 +912,14 @@ top: ASSERT(zp->z_phys->zp_xattr == 0); -#ifdef TODO if (!(flags & CREATE_XATTR_DIR)) { zfs_dirent_unlock(dl); +#ifdef __FreeBSD__ + return (ENOATTR); +#else return (ENOENT); - } #endif + } if (zfsvfs->z_vfs->vfs_flag & VFS_RDONLY) { zfs_dirent_unlock(dl); Modified: stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c ============================================================================== --- stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Fri Jan 22 11:42:23 2010 (r202804) +++ stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Fri Jan 22 11:42:44 2010 (r202805) @@ -4537,6 +4537,11 @@ vop_getextattr { vnode_t *xvp = NULL, *vp; int error, flags; + error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, + ap->a_cred, ap->a_td, VREAD); + if (error != 0) + return (error); + error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, sizeof(attrname)); if (error != 0) @@ -4558,6 +4563,8 @@ vop_getextattr { vp = nd.ni_vp; NDFREE(&nd, NDF_ONLY_PNBUF); if (error != 0) { + if (error == ENOENT) + error = ENOATTR; ZFS_EXIT(zfsvfs); return (error); } @@ -4599,6 +4606,11 @@ vop_deleteextattr { vnode_t *xvp = NULL, *vp; int error, flags; + error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, + ap->a_cred, ap->a_td, VWRITE); + if (error != 0) + return (error); + error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, sizeof(attrname)); if (error != 0) @@ -4619,6 +4631,8 @@ vop_deleteextattr { vp = nd.ni_vp; NDFREE(&nd, NDF_ONLY_PNBUF); if (error != 0) { + if (error == ENOENT) + error = ENOATTR; ZFS_EXIT(zfsvfs); return (error); } @@ -4658,6 +4672,11 @@ vop_setextattr { vnode_t *xvp = NULL, *vp; int error, flags; + error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, + ap->a_cred, ap->a_td, VWRITE); + if (error != 0) + return (error); + error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, sizeof(attrname)); if (error != 0) @@ -4666,7 +4685,7 @@ vop_setextattr { ZFS_ENTER(zfsvfs); error = zfs_lookup(ap->a_vp, NULL, &xvp, NULL, 0, ap->a_cred, td, - LOOKUP_XATTR); + LOOKUP_XATTR | CREATE_XATTR_DIR); if (error != 0) { ZFS_EXIT(zfsvfs); return (error); @@ -4725,6 +4744,11 @@ vop_listextattr { vnode_t *xvp = NULL, *vp; int done, error, eof, pos; + error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, + ap->a_cred, ap->a_td, VREAD); + if (error) + return (error); + error = zfs_create_attrname(ap->a_attrnamespace, "", attrprefix, sizeof(attrprefix)); if (error != 0) @@ -4739,6 +4763,12 @@ vop_listextattr { error = zfs_lookup(ap->a_vp, NULL, &xvp, NULL, 0, ap->a_cred, td, LOOKUP_XATTR); if (error != 0) { + /* + * ENOATTR means that the EA directory does not yet exist, + * i.e. there are no extended attributes there. + */ + if (error == ENOATTR) + error = 0; ZFS_EXIT(zfsvfs); return (error); }