changeset 10143:d2d432dfe597 onnv_120

6857433 memory leaks found at: zfs_acl_alloc/zfs_acl_node_alloc 6860318 truncate() on zfsroot succeeds when file has a component of its path set without access permission
author Tim Haley <Tim.Haley@Sun.COM>
date Mon, 20 Jul 2009 19:57:58 -0600
parents 40fdaae7c699
children 445322d1cb7d
files usr/src/uts/common/fs/zfs/zfs_acl.c usr/src/uts/common/fs/zfs/zfs_vnops.c usr/src/uts/common/fs/zfs/zfs_znode.c
diffstat 3 files changed, 32 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/fs/zfs/zfs_acl.c	Mon Jul 20 17:19:02 2009 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_acl.c	Mon Jul 20 19:57:58 2009 -0600
@@ -93,6 +93,8 @@
 #define	ZFS_ACL_WIDE_FLAGS (V4_ACL_WIDE_FLAGS|ZFS_ACL_TRIVIAL|ZFS_INHERIT_ACE|\
     ZFS_ACL_OBJ_ACE)
 
+#define	ALL_MODE_EXECS (S_IXUSR | S_IXGRP | S_IXOTH)
+
 static uint16_t
 zfs_ace_v0_get_type(void *acep)
 {
@@ -917,8 +919,14 @@
 		}
 	}
 
-	if (!an_exec_denied && !(seen & (S_IXUSR | S_IXGRP | S_IXOTH)) ||
-	    !(mode & (S_IXUSR | S_IXGRP | S_IXOTH)))
+	/*
+	 * Failure to allow is effectively a deny, so execute permission
+	 * is denied if it was never mentioned or if we explicitly
+	 * weren't allowed it.
+	 */
+	if (!an_exec_denied &&
+	    ((seen & ALL_MODE_EXECS) != ALL_MODE_EXECS ||
+	    (mode & ALL_MODE_EXECS) != ALL_MODE_EXECS))
 		an_exec_denied = B_TRUE;
 
 	if (an_exec_denied)
@@ -965,7 +973,8 @@
 }
 
 /*
- * Read an external acl object.
+ * Read an external acl object.  If the intent is to modify, always
+ * create a new acl and leave any cached acl in place.
  */
 static int
 zfs_acl_node_read(znode_t *zp, zfs_acl_t **aclpp, boolean_t will_modify)
@@ -979,14 +988,15 @@
 
 	ASSERT(MUTEX_HELD(&zp->z_acl_lock));
 
-	if (zp->z_acl_cached) {
+	if (zp->z_acl_cached && !will_modify) {
 		*aclpp = zp->z_acl_cached;
 		return (0);
 	}
 
 	if (zp->z_phys->zp_acl.z_acl_extern_obj == 0) {
 		*aclpp = zfs_acl_node_read_internal(zp, will_modify);
-		zp->z_acl_cached = *aclpp;
+		if (!will_modify)
+			zp->z_acl_cached = *aclpp;
 		return (0);
 	}
 
@@ -1019,7 +1029,9 @@
 		return (error);
 	}
 
-	zp->z_acl_cached = *aclpp = aclp;
+	*aclpp = aclp;
+	if (!will_modify)
+		zp->z_acl_cached = aclp;
 	return (0);
 }
 
@@ -1044,7 +1056,7 @@
 
 	dmu_buf_will_dirty(zp->z_dbuf, tx);
 
-	if (zp->z_acl_cached != aclp && zp->z_acl_cached) {
+	if (zp->z_acl_cached) {
 		zfs_acl_free(zp->z_acl_cached);
 		zp->z_acl_cached = NULL;
 	}
@@ -1052,8 +1064,8 @@
 	zphys->zp_mode = zfs_mode_compute(zp, aclp);
 
 	/*
-	 * Decide which opbject type to use.  If we are forced to
-	 * use old ACL format than transform ACL into zfs_oldace_t
+	 * Decide which object type to use.  If we are forced to
+	 * use old ACL format then transform ACL into zfs_oldace_t
 	 * layout.
 	 */
 	if (!zfsvfs->z_use_fuids) {
@@ -1636,7 +1648,6 @@
 	if (error == 0) {
 		(*aclp)->z_hints = zp->z_phys->zp_flags & V4_ACL_WIDE_FLAGS;
 		zfs_acl_chmod(zp->z_zfsvfs, zp->z_phys->zp_uid, mode, *aclp);
-		zp->z_acl_cached = *aclp;
 	}
 	mutex_exit(&zp->z_acl_lock);
 	mutex_exit(&zp->z_lock);
@@ -2168,6 +2179,7 @@
 
 	error = zfs_aclset_common(zp, aclp, cr, tx);
 	ASSERT(error == 0);
+	zp->z_acl_cached = aclp;
 
 	if (fuid_dirtied)
 		zfs_fuid_sync(zfsvfs, tx);
@@ -2177,7 +2189,6 @@
 
 	if (fuidp)
 		zfs_fuid_info_free(fuidp);
-	zp->z_acl_cached = aclp;
 	dmu_tx_commit(tx);
 done:
 	mutex_exit(&zp->z_acl_lock);
@@ -2343,7 +2354,6 @@
 				    uint32_t, mask_matched);
 				if (anyaccess) {
 					mutex_exit(&zp->z_acl_lock);
-					zfs_acl_free(aclp);
 					return (0);
 				}
 			}
--- a/usr/src/uts/common/fs/zfs/zfs_vnops.c	Mon Jul 20 17:19:02 2009 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c	Mon Jul 20 19:57:58 2009 -0600
@@ -1282,6 +1282,7 @@
 		    &acl_ids)) != 0)
 			goto out;
 		if (zfs_acl_ids_overquota(zfsvfs, &acl_ids)) {
+			zfs_acl_ids_free(&acl_ids);
 			error = EDQUOT;
 			goto out;
 		}
@@ -1688,6 +1689,7 @@
 		return (error);
 	}
 	if (zfs_acl_ids_overquota(zfsvfs, &acl_ids)) {
+		zfs_acl_ids_free(&acl_ids);
 		zfs_dirent_unlock(dl);
 		ZFS_EXIT(zfsvfs);
 		return (EDQUOT);
@@ -2801,6 +2803,8 @@
 		zp->z_phys->zp_mode = new_mode;
 		err = zfs_aclset_common(zp, aclp, cr, tx);
 		ASSERT3U(err, ==, 0);
+		zp->z_acl_cached = aclp;
+		aclp = NULL;
 		mutex_exit(&zp->z_acl_lock);
 	}
 
@@ -2892,6 +2896,9 @@
 	if (attrzp)
 		VN_RELE(ZTOV(attrzp));
 
+	if (aclp)
+		zfs_acl_free(aclp);
+
 	if (fuidp) {
 		zfs_fuid_info_free(fuidp);
 		fuidp = NULL;
--- a/usr/src/uts/common/fs/zfs/zfs_znode.c	Mon Jul 20 17:19:02 2009 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_znode.c	Mon Jul 20 19:57:58 2009 -0600
@@ -156,6 +156,7 @@
 
 	ASSERT(zp->z_dbuf == NULL);
 	ASSERT(zp->z_dirlocks == NULL);
+	ASSERT(zp->z_acl_cached == NULL);
 }
 
 #ifdef	ZNODE_STATS
@@ -199,6 +200,7 @@
 	nzp->z_sync_cnt = ozp->z_sync_cnt;
 	nzp->z_phys = ozp->z_phys;
 	nzp->z_dbuf = ozp->z_dbuf;
+	nzp->z_acl_cached = ozp->z_acl_cached;
 
 	/* Update back pointers. */
 	(void) dmu_buf_update_user(nzp->z_dbuf, ozp, nzp, &nzp->z_phys,
@@ -211,6 +213,7 @@
 	 * subsequent callback.
 	 */
 	ozp->z_dbuf = NULL;
+	ozp->z_acl_cached = NULL;
 	POINTER_INVALIDATE(&ozp->z_zfsvfs);
 }