Date: Wed, 14 Oct 2009 11:18:23 GMT From: Gleb Kurtsou <gk@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 169484 for review Message-ID: <200910141118.n9EBINbn010315@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/chv.cgi?CH=169484 Change 169484 by gk@gk_h1 on 2009/10/14 11:17:55 introduce pefs tests (runs tests on top of tmpfs) various fixes spotted by tests: pefs_lookup, pefs_rename, pefs_write add pefs showkeys -t option (only test. do not print) fix i386 warning Affected files ... .. //depot/projects/soc2009/gk_pefs/sbin/pefs/pefs_ctl.c#11 edit .. //depot/projects/soc2009/gk_pefs/sys/fs/pefs/pefs_crypto.c#16 edit .. //depot/projects/soc2009/gk_pefs/sys/fs/pefs/pefs_subr.c#17 edit .. //depot/projects/soc2009/gk_pefs/sys/fs/pefs/pefs_vnops.c#23 edit .. //depot/projects/soc2009/gk_pefs/sys/fs/pefs/vmac.c#2 edit .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/Makefile#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/h_funcs.subr#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/h_tools.c#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_create#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_dots#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_exec#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_link#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_mkdir#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_mount#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_pipes#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_read_write#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_readdir#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_remove#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_rename#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_rmdir#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_setattr#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_sizes#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_sockets#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_statvfs#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_symlink#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_times#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_trail_slash#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_truncate#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_vnd#1 add .. //depot/projects/soc2009/gk_pefs/tools/regression/pefs/t_vnode_leak#1 add Differences ... ==== //depot/projects/soc2009/gk_pefs/sbin/pefs/pefs_ctl.c#11 (text+ko) ==== @@ -407,10 +407,14 @@ pefs_showkeys(int argc, char *argv[]) { struct pefs_xkey k; + int testonly = 0; int fd, i; - while ((i = getopt(argc, argv, "")) != -1) + while ((i = getopt(argc, argv, "t")) != -1) switch(i) { + case 't': + testonly = 1; + break; case '?': default: pefs_usage(); @@ -428,11 +432,19 @@ bzero(&k, sizeof(k)); if (ioctl(fd, PEFS_GETKEY, &k) == -1) { + if (testonly) { + close(fd); + return (EX_DATAERR); + } if (errno == ENOENT) printf("No keys specified\n"); else err(EX_IOERR, "cannot list keys"); } else { + if (testonly) { + close(fd); + return (0); + } printf("Keys:\n"); while (1) { pefs_key_show(&k, k.pxk_index); @@ -833,7 +845,7 @@ " pefs setkey [-cCpvx] [-a alg] [-i iterations] [-k keyfile] directory\n" " pefs delkey [-cCpv] [-i iterations] [-k keyfile] filesystem\n" " pefs flushkeys filesystem\n" -" pefs showkeys filesystem\n" +" pefs showkeys [-t] filesystem\n" " pefs addchain [-pPvZ] [-a alg] [-i iterations] [-k keyfile]\n" " [-A alg] [-I iterations] [-K keyfile] filesystem\n" " pefs delchain [-pv] [-i iterations] [-k keyfile] filesystem\n" ==== //depot/projects/soc2009/gk_pefs/sys/fs/pefs/pefs_crypto.c#16 (text+ko) ==== @@ -558,8 +558,8 @@ return (-ENAMETOOLONG); } if (enc_size < r) { - printf("pefs: name encryption buffer is too small: length %ld, required %d\n", - enc_size, r); + printf("pefs: name encryption buffer is too small: length %jd, required %d\n", + (intmax_t)enc_size, r); return (-EOVERFLOW); } @@ -610,8 +610,8 @@ (r - PEFS_NAME_CSUM_SIZE) % PEFS_NAME_BLOCK_SIZE != 0) return (-EINVAL); if (plain_size < r) { - printf("pefs: name decryption buffer is too small: length %ld, required %d\n", - plain_size, r); + printf("pefs: name decryption buffer is too small: length %jd, required %d\n", + (intmax_t)plain_size, r); return (-EOVERFLOW); } ==== //depot/projects/soc2009/gk_pefs/sys/fs/pefs/pefs_subr.c#17 (text+ko) ==== @@ -561,7 +561,7 @@ if (size > DFLTPHYS) panic("pefs_chunk_create: requested buffer is too large %jd", - size); + (intmax_t)size); nodebuf = 0; wantbufsize = (size <= PAGE_SIZE ? PAGE_SIZE : DFLTPHYS); ==== //depot/projects/soc2009/gk_pefs/sys/fs/pefs/pefs_vnops.c#23 (text+ko) ==== @@ -458,8 +458,8 @@ int nokey_lookup, skip_lookup; int error; - PEFSDEBUG("pefs_lookup: cn_nameiop=%lx, cn_nameptr=%.*s %p\n", - cnp->cn_nameiop, (int)cnp->cn_namelen, cnp->cn_nameptr, cnp->cn_nameptr); + PEFSDEBUG("pefs_lookup: op=%lx, name=%.*s\n", + cnp->cn_nameiop, (int)cnp->cn_namelen, cnp->cn_nameptr); pefs_enccn_init(&enccn); @@ -474,8 +474,6 @@ if (((dpn->pn_flags & PN_HASKEY) == 0 || skip_lookup)) { error = VOP_LOOKUP(ldvp, &lvp, cnp); if (skip_lookup || error == 0 || - (error == EJUSTRETURN && - !((flags & ISLASTCN) && cnp->cn_nameiop == CREATE)) || pefs_no_keys(dvp)) nokey_lookup = 1; } @@ -557,8 +555,9 @@ if (!nokey_lookup) pefs_enccn_free(&enccn); - PEFSDEBUG("pefs_lookup: cn_nameiop=%lx, cn_nameptr=%.*s %p\n", - cnp->cn_nameiop, (int)cnp->cn_namelen, cnp->cn_nameptr, cnp->cn_nameptr); + PEFSDEBUG("pefs_lookup: op=%lx, name=%.*s error=%d vp=%p\n", + cnp->cn_nameiop, (int)cnp->cn_namelen, cnp->cn_nameptr, error, + *ap->a_vpp); return (error); } @@ -639,7 +638,7 @@ pefs_data_encrypt_update(ctx, &pn->pn_tkey, &pc); puio = pefs_chunk_uio(&pc, offset, UIO_WRITE); PEFSDEBUG("pefs_tryextend: resizing file; filling with zeros: offset=0x%jx, resid=0x%jx\n", - offset, bsize); + offset, (intmax_t)bsize); error = VOP_WRITE(lvp, puio, 0, cred); if (error != 0) { /* try to reset */ @@ -798,6 +797,19 @@ } static int +pefs_close(struct vop_close_args *ap) +{ + struct vnode *vp = ap->a_vp; + struct vnode *lvp = PEFS_LOWERVP(vp); + int error; + + ap->a_vp = lvp; + error = VOP_CLOSE_AP(ap); + ap->a_vp = vp; + return (error); +} + +static int pefs_rename(struct vop_rename_args *ap) { struct vnode *fdvp = ap->a_fdvp; @@ -808,7 +820,6 @@ struct vnode *ltdvp = PEFS_LOWERVP(tdvp); struct vnode *tvp = ap->a_tvp; struct vnode *ltvp = (tvp == NULL ? NULL : PEFS_LOWERVP(tvp)); - struct vnode *tovp = NULL; struct componentname *fcnp = ap->a_fcnp; struct componentname *tcnp = ap->a_tcnp; struct pefs_enccn fenccn; @@ -826,6 +837,14 @@ goto bad; } + /* Handle '.' and '..' rename attempt */ + if ((fcnp->cn_namelen == 1 && fcnp->cn_nameptr[0] == '.') || + (fcnp->cn_flags & ISDOTDOT) || (tcnp->cn_flags & ISDOTDOT) || + fdvp == fvp) { + error = EINVAL; + goto bad; + } + if (pefs_no_keys(tdvp)) { error = EROFS; goto bad; @@ -859,55 +878,108 @@ if (error != 0) { goto bad; } - error = pefs_enccn_create(&tenccn, fenccn.pec_tkey.ptk_key, - fenccn.pec_tkey.ptk_tweak, tcnp); - if (error != 0) { - pefs_enccn_free(&fenccn); - goto bad; - } +#ifdef PEFS_DEBUG_EXTRA if (tvp == NULL) { tcnp->cn_nameiop = DELETE; - error = VOP_LOOKUP(tdvp, &tovp, tcnp); + error = VOP_LOOKUP(tdvp, &tvp, tcnp); tcnp->cn_nameiop = RENAME; - PEFSDEBUG("pefs_rename: lookup target vnode: %s: error=%d, tovp=%p\n", - tcnp->cn_nameptr, error, tovp); - if (error == ENOENT) + MPASS(error != 0); + if (error == ENOENT) { error = 0; + } else { + PEFSDEBUG("pefs_rename: lookup target vnode: %s: error=%d, tvp=%p\n", + tcnp->cn_nameptr, error, tvp); + if (error == 0) + vput(tvp); + tvp = NULL; + error = EINVAL; + pefs_enccn_free(&fenccn); + goto bad; + } } +#endif + if (tvp != NULL) { + if (fvp->v_type == VDIR && tvp->v_type == VDIR) { + /* + * Lower level is to check if tvp directory is empty. + * After rename fvp will contain invalid key/tweak + * because it is rename of fvp. + */ + error = pefs_enccn_get(&tenccn, tvp, tcnp); + } else if (fvp->v_type == VDIR && tvp->v_type != VDIR) { + error = ENOTDIR; + } else if (fvp->v_type != VDIR && tvp->v_type == VDIR) { + error = EISDIR; + } else { + /* + * (fvp->v_type != VDIR && tvp->v_type != VDIR): + * We end up having 2 files with same name but + * different tweaks/keys. Remove the old one. + * Set ltvp to zero here because we rename to new name + * and then remove old one + */ + error = pefs_enccn_create(&tenccn, + fenccn.pec_tkey.ptk_key, fenccn.pec_tkey.ptk_tweak, + tcnp); + if (error == 0) + ltvp = NULL; + } + } else { + error = pefs_enccn_create(&tenccn, fenccn.pec_tkey.ptk_key, + fenccn.pec_tkey.ptk_tweak, tcnp); + } if (error != 0) { pefs_enccn_free(&fenccn); - pefs_enccn_free(&tenccn); goto bad; } vref(lfdvp); vref(lfvp); vref(ltdvp); - if (ltvp != NULL) + ASSERT_VOP_LOCKED(ltdvp, "pefs_rename"); + if (ltvp != NULL) { + ASSERT_VOP_LOCKED(ltvp, "pefs_rename"); vref(ltvp); + } error = VOP_RENAME(lfdvp, lfvp, &fenccn.pec_cn, ltdvp, ltvp, &tenccn.pec_cn); - pefs_enccn_free(&fenccn); - pefs_enccn_free(&tenccn); - if (error == 0) { - if (tovp != NULL) { - MPASS(tovp->v_type == VREG); + if (tvp != NULL && tvp->v_type != VDIR) { + /* + * Remove old file. Double rename is not performed to + * save data in case of error + */ + pefs_enccn_free(&tenccn); + ASSERT_VOP_UNLOCKED(tdvp, "pefs_rename"); + ASSERT_VOP_LOCKED(tvp, "pefs_rename"); vn_lock(tdvp, LK_EXCLUSIVE | LK_RETRY); - tcnp->cn_nameiop = DELETE; - error = VOP_REMOVE(tdvp, tovp, tcnp); - tcnp->cn_nameiop = RENAME; + error = pefs_enccn_get(&tenccn, tvp, tcnp); + if (error == 0) { + error = VOP_REMOVE(ltdvp, PEFS_LOWERVP(tvp), + &tenccn.pec_cn); + PEFSDEBUG("pefs_rename: remove old: %s\n", + tenccn.pec_cn.cn_nameptr); + VP_TO_PN(tvp)->pn_flags |= PN_WANTRECYCLE; + cache_purge(tvp); + vgone(tvp); + } VOP_UNLOCK(tdvp, 0); - vput(tovp); + VOP_UNLOCK(tvp, 0); } cache_purge(fdvp); cache_purge(fvp); - } else { - if (tovp != NULL) - vput(tovp); + if (tvp != NULL && tvp->v_type == VDIR) { + vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY); + VP_TO_PN(fvp)->pn_flags |= PN_WANTRECYCLE; + vgone(fvp); + VOP_UNLOCK(fvp, 0); + } } + pefs_enccn_free(&tenccn); + pefs_enccn_free(&fenccn); + done: ASSERT_VOP_UNLOCKED(tdvp, "pefs_rename"); vrele(fdvp); @@ -1691,7 +1763,7 @@ vm_page_busy(m); VM_OBJECT_UNLOCK(vp->v_object); PEFSDEBUG("pefs_read: mapped: offset=0x%jx moffset=0x%jx msize=0x%jx\n", - uio->uio_offset, moffset, msize); + uio->uio_offset, (intmax_t)moffset, (intmax_t)msize); error = uiomove_fromphys(&m, moffset, msize, uio); VM_OBJECT_LOCK(vp->v_object); vm_page_wakeup(m); @@ -1711,7 +1783,7 @@ } PEFSDEBUG("pefs_read: mapped=%d m=%d offset=0x%jx size=0x%jx\n", - mapped, m != NULL, uio->uio_offset, pc.pc_size); + mapped, m != NULL, uio->uio_offset, (intmax_t)pc.pc_size); puio = pefs_chunk_uio(&pc, uio->uio_offset, uio->uio_rw); error = VOP_READ(lvp, puio, ioflag, cred); if (error != 0) @@ -1829,7 +1901,7 @@ vm_page_unlock_queues(); VM_OBJECT_UNLOCK(vp->v_object); PEFSDEBUG("pefs_write: mapped: offset=0x%jx moffset=0x%jx msize=0x%jx\n", - offset, moffset, msize); + offset, (intmax_t)moffset, (intmax_t)msize); sched_pin(); sf = sf_buf_alloc(m, SFB_CPUPRIVATE); ma = (char *)sf_buf_kva(sf); @@ -1866,7 +1938,7 @@ pefs_chunk_copy(&pc, uio); lower_update: PEFSDEBUG("pefs_write: mapped=%d m=%d offset=0x%jx size=0x%jx\n", - mapped, m != NULL, offset, pc.pc_size); + mapped, m != NULL, offset, (intmax_t)pc.pc_size); if (restart_encrypt) { restart_encrypt = 0; pefs_data_encrypt_start(ctx, &pn->pn_tkey, offset); @@ -1876,13 +1948,29 @@ /* IO_APPEND handled above to prevent offset change races. */ error = VOP_WRITE(lvp, puio, ioflag, cred); - if (error != 0) + if (error != 0) { + /* + * XXX Original uio is not preserved thus can't be + * restored. Cloning entire uio structure is too + * inefficient. uio shouldn't be reused after error, but + * uid_resid and uio_offset should remain valid. + * Partially destroy uio, so that using it further + * would result in panic. + * + * Correct solution is to implement uiocopy that + * doesn't change uio and iovec. + */ + uio->uio_iov = NULL; + uio->uio_iovcnt = 0; + uio->uio_rw = -1; + uio->uio_resid += pc.pc_size; + uio->uio_offset -= pc.pc_size; break; + } MPASS(puio->uio_resid == 0); resid -= pc.pc_size; offset += pc.pc_size; - } pefs_ctx_free(ctx); pefs_chunk_free(&pc, pn); @@ -2085,6 +2173,7 @@ .vop_default = &default_vnodeops, .vop_access = pefs_access, .vop_accessx = pefs_accessx, + .vop_close = pefs_close, .vop_getattr = pefs_getattr, .vop_getwritemount = pefs_getwritemount, .vop_inactive = pefs_inactive, ==== //depot/projects/soc2009/gk_pefs/sys/fs/pefs/vmac.c#2 (text+ko) ==== @@ -251,17 +251,17 @@ #define a1 *(((uint32_t*)alo)+INDEX_HIGH) #define a2 *(((uint32_t*)ahi)+INDEX_LOW) #define a3 *(((uint32_t*)ahi)+INDEX_HIGH) -#define k0 *(((uint32_t*)kl)+INDEX_LOW) -#define k1 *(((uint32_t*)kl)+INDEX_HIGH) -#define k2 *(((uint32_t*)kh)+INDEX_LOW) -#define k3 *(((uint32_t*)kh)+INDEX_HIGH) +#define k0 *(((const uint32_t*)kl)+INDEX_LOW) +#define k1 *(((const uint32_t*)kl)+INDEX_HIGH) +#define k2 *(((const uint32_t*)kh)+INDEX_LOW) +#define k3 *(((const uint32_t*)kh)+INDEX_HIGH) uint64_t p, q, t; uint32_t t2; p = MUL32(a3, k3); p += p; - p += *(uint64_t *)mh; + p += *(const uint64_t *)mh; p += MUL32(a0, k2); p += MUL32(a1, k1); p += MUL32(a2, k0); @@ -273,7 +273,7 @@ p += MUL32(a3, k0); t |= ((uint64_t)((uint32_t)p & 0x7fffffff)) << 32; p >>= 31; - p += (uint64_t)(((uint32_t*)ml)[INDEX_LOW]); + p += (uint64_t)(((const uint32_t*)ml)[INDEX_LOW]); p += MUL32(a0, k0); q = MUL32(a1, k3); q += MUL32(a2, k2); @@ -282,7 +282,7 @@ p += q; t2 = (uint32_t)(p); p >>= 32; - p += (uint64_t)(((uint32_t*)ml)[INDEX_HIGH]); + p += (uint64_t)(((const uint32_t*)ml)[INDEX_HIGH]); p += MUL32(a0, k1); p += MUL32(a1, k0); q = MUL32(a2, k3);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200910141118.n9EBINbn010315>