Mercurial > illumos > illumos-gate
changeset 13101:08bbd228b732
6975482 assertion failed: error == 0 (0x2 == 0x0), file: ../../common/fs/zfs/zfs_vnops.c, line: 1630
6975131 zfs_remove leaks xattr vnode holds, which can lead to zone halt problem
6975190 memory leak after zfs stress testing
| author | Mark Shellenbaum <Mark.Shellenbaum@Oracle.COM> |
|---|---|
| date | Thu, 12 Aug 2010 13:32:15 -0600 |
| parents | 2569f04244aa |
| children | 579992f451e3 |
| files | usr/src/uts/common/fs/zfs/sys/zfs_acl.h usr/src/uts/common/fs/zfs/zfs_acl.c usr/src/uts/common/fs/zfs/zfs_vnops.c |
| diffstat | 3 files changed, 34 insertions(+), 15 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/uts/common/fs/zfs/sys/zfs_acl.h Thu Aug 12 08:33:09 2010 -0700 +++ b/usr/src/uts/common/fs/zfs/sys/zfs_acl.h Thu Aug 12 13:32:15 2010 -0600 @@ -218,7 +218,7 @@ extern int zfs_zaccess_rwx(struct znode *, mode_t, int, cred_t *); extern int zfs_zaccess_unix(struct znode *, mode_t, cred_t *); extern int zfs_acl_access(struct znode *, int, cred_t *); -int zfs_acl_chmod_setattr(struct znode *, zfs_acl_t **, uint64_t); +void zfs_acl_chmod_setattr(struct znode *, zfs_acl_t **, uint64_t); int zfs_zaccess_delete(struct znode *, struct znode *, cred_t *); int zfs_zaccess_rename(struct znode *, struct znode *, struct znode *, struct znode *, cred_t *cr);
--- a/usr/src/uts/common/fs/zfs/zfs_acl.c Thu Aug 12 08:33:09 2010 -0700 +++ b/usr/src/uts/common/fs/zfs/zfs_acl.c Thu Aug 12 13:32:15 2010 -0600 @@ -1505,7 +1505,7 @@ list_insert_tail(&aclp->z_acl, newnode); } -int +void zfs_acl_chmod_setattr(znode_t *zp, zfs_acl_t **aclp, uint64_t mode) { mutex_enter(&zp->z_acl_lock); @@ -1516,7 +1516,6 @@ mutex_exit(&zp->z_lock); mutex_exit(&zp->z_acl_lock); ASSERT(*aclp); - return (0); } /* @@ -2060,6 +2059,7 @@ error = zfs_aclset_common(zp, aclp, cr, tx); ASSERT(error == 0); + ASSERT(zp->z_acl_cached == NULL); zp->z_acl_cached = aclp; if (fuid_dirtied)
--- a/usr/src/uts/common/fs/zfs/zfs_vnops.c Thu Aug 12 08:33:09 2010 -0700 +++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c Thu Aug 12 13:32:15 2010 -0600 @@ -1369,6 +1369,8 @@ error = zfs_dirent_lock(&dl, dzp, name, &zp, zflg, NULL, NULL); if (error) { + if (have_acl) + zfs_acl_ids_free(&acl_ids); if (strcmp(name, "..") == 0) error = EISDIR; ZFS_EXIT(zfsvfs); @@ -1384,6 +1386,8 @@ * to reference it. */ if (error = zfs_zaccess(dzp, ACE_ADD_FILE, 0, B_FALSE, cr)) { + if (have_acl) + zfs_acl_ids_free(&acl_ids); goto out; } @@ -1394,6 +1398,8 @@ if ((dzp->z_pflags & ZFS_XATTR) && (vap->va_type != VREG)) { + if (have_acl) + zfs_acl_ids_free(&acl_ids); error = EINVAL; goto out; } @@ -1453,6 +1459,10 @@ } else { int aflags = (flag & FAPPEND) ? V_APPEND : 0; + if (have_acl) + zfs_acl_ids_free(&acl_ids); + have_acl = B_FALSE; + /* * A directory entry already exists for this name. */ @@ -1540,11 +1550,11 @@ int flags) { znode_t *zp, *dzp = VTOZ(dvp); - znode_t *xzp = NULL; + znode_t *xzp; vnode_t *vp; zfsvfs_t *zfsvfs = dzp->z_zfsvfs; zilog_t *zilog; - uint64_t acl_obj, xattr_obj = 0; + uint64_t acl_obj, xattr_obj; uint64_t xattr_obj_unlinked = 0; uint64_t obj = 0; zfs_dirlock_t *dl; @@ -1568,6 +1578,8 @@ } top: + xattr_obj = 0; + xzp = NULL; /* * Attempt to lock directory; fail if entry doesn't exist. */ @@ -1627,7 +1639,7 @@ /* are there any extended attributes? */ error = sa_lookup(zp->z_sa_hdl, SA_ZPL_XATTR(zfsvfs), &xattr_obj, sizeof (xattr_obj)); - if (xattr_obj) { + if (error == 0 && xattr_obj) { error = zfs_zget(zfsvfs, xattr_obj, &xzp); ASSERT3U(error, ==, 0); dmu_tx_hold_sa(tx, zp->z_sa_hdl, B_TRUE); @@ -1646,6 +1658,8 @@ if (error) { zfs_dirent_unlock(dl); VN_RELE(vp); + if (xzp) + VN_RELE(ZTOV(xzp)); if (error == ERESTART) { dmu_tx_wait(tx); dmu_tx_abort(tx); @@ -2610,7 +2624,7 @@ int trim_mask = 0; uint64_t new_mode; uint64_t new_uid, new_gid; - uint64_t xattr_obj = 0; + uint64_t xattr_obj; uint64_t mtime[2], ctime[2]; znode_t *attrzp; int need_policy = FALSE; @@ -2618,7 +2632,7 @@ zfs_fuid_info_t *fuidp = NULL; xvattr_t *xvap = (xvattr_t *)vap; /* vap may be an xvattr_t * */ xoptattr_t *xoap; - zfs_acl_t *aclp = NULL; + zfs_acl_t *aclp; boolean_t skipaclchk = (flags & ATTR_NOACLCHECK) ? B_TRUE : B_FALSE; boolean_t fuid_dirtied = B_FALSE; sa_bulk_attr_t bulk[7], xattr_bulk[7]; @@ -2697,6 +2711,7 @@ top: attrzp = NULL; + aclp = NULL; /* Can this be moved to before the top label? */ if (zfsvfs->z_vfs->vfs_flag & VFS_RDONLY) { @@ -2921,10 +2936,10 @@ mask = vap->va_mask; if ((mask & (AT_UID | AT_GID))) { - (void) sa_lookup(zp->z_sa_hdl, SA_ZPL_XATTR(zfsvfs), &xattr_obj, - sizeof (xattr_obj)); - - if (xattr_obj) { + err = sa_lookup(zp->z_sa_hdl, SA_ZPL_XATTR(zfsvfs), + &xattr_obj, sizeof (xattr_obj)); + + if (err == 0 && xattr_obj) { err = zfs_zget(zp->z_zfsvfs, xattr_obj, &attrzp); if (err) goto out2; @@ -2934,6 +2949,8 @@ (uint64_t)vap->va_uid, cr, ZFS_OWNER, &fuidp); if (new_uid != zp->z_uid && zfs_fuid_overquota(zfsvfs, B_FALSE, new_uid)) { + if (attrzp) + VN_RELE(ZTOV(attrzp)); err = EDQUOT; goto out2; } @@ -2944,6 +2961,8 @@ cr, ZFS_GROUP, &fuidp); if (new_gid != zp->z_gid && zfs_fuid_overquota(zfsvfs, B_TRUE, new_gid)) { + if (attrzp) + VN_RELE(ZTOV(attrzp)); err = EDQUOT; goto out2; } @@ -2956,8 +2975,7 @@ uint64_t acl_obj; new_mode = (pmode & S_IFMT) | (vap->va_mode & ~S_IFMT); - if (err = zfs_acl_chmod_setattr(zp, &aclp, new_mode)) - goto out; + zfs_acl_chmod_setattr(zp, &aclp, new_mode); mutex_enter(&zp->z_lock); if (!zp->z_is_sa && ((acl_obj = zfs_external_acl(zp)) != 0)) { @@ -3078,6 +3096,8 @@ ASSERT3U((uintptr_t)aclp, !=, NULL); err = zfs_aclset_common(zp, aclp, cr, tx); ASSERT3U(err, ==, 0); + if (zp->z_acl_cached) + zfs_acl_free(zp->z_acl_cached); zp->z_acl_cached = aclp; aclp = NULL; } @@ -3194,7 +3214,6 @@ dmu_tx_commit(tx); } - out2: if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS) zil_commit(zilog, 0);