Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 22 Jan 2010 11:42:44 +0000 (UTC)
From:      Edward Tomasz Napierala <trasz@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r202805 - stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <201001221142.o0MBgi5l058119@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
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);
 	}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201001221142.o0MBgi5l058119>