Date: Thu, 6 Aug 2009 06:56:30 GMT From: Tatsiana Severyna <tatsianka@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 167058 for review Message-ID: <200908060656.n766uUTe089775@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=167058 Change 167058 by tatsianka@tatsianka_zonder on 2009/08/06 06:55:53 puffs_vnop_pathconf: fix usage after free puffs_vnop_fsync: do not unlock/lock vnode if faf is set Nuke DTRACE. Reduce DPRINTF's I've added while porting. Comments Affected files ... .. //depot/projects/soc2009/tatsianka_puffs/libpuffs/callcontext.c#5 edit .. //depot/projects/soc2009/tatsianka_puffs/libpuffs/null.c#5 edit .. //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_msgif.c#7 edit .. //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_node.c#9 edit .. //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_subr.c#6 edit .. //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_sys.h#6 edit .. //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_vfsops.c#8 edit .. //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_vnops.c#9 edit .. //depot/projects/soc2009/tatsianka_puffs/putter/putter.c#6 edit Differences ... ==== //depot/projects/soc2009/tatsianka_puffs/libpuffs/callcontext.c#5 (text+ko) ==== @@ -47,7 +47,7 @@ #include "puffs_priv.h" -#if 1 +#if 0 #define DPRINTF(x) printf x #else #define DPRINTF(x) @@ -186,7 +186,10 @@ if (puffs_fakecc) return &fakecc; - /* XXX_TS MAP_ALIGNED(pu->pu_cc_stackshift) */ + /* + * Emulate MAP_ALIGNED(pu->pu_cc_stackshift) by allocating stacksize*2 + * bytes and unmapping extra pages + */ sp = mmap(NULL, stacksize * 2, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, -1, 0); if (sp == MAP_FAILED) ==== //depot/projects/soc2009/tatsianka_puffs/libpuffs/null.c#5 (text+ko) ==== @@ -207,7 +207,7 @@ * * Yes, this really really needs fixing. Yes, *REALLY*. * - * XXX_TS: Directly use struct fid, as fh larger then MAXFIDSZ is unsupported + * FreeBSD: Use struct fid, as file handles larger then MAXFIDSZ are unsupported */ /*ARGSUSED*/ ==== //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_msgif.c#7 (text+ko) ==== @@ -238,7 +238,6 @@ struct puffs_msgpark *park; void *m; - /* XXX check if M_NOWAIT if needed */ /* XXX_TS cansleep is always true */ m = malloc(len, M_PUFFS, (cansleep ? M_NOWAIT : M_WAITOK) | M_ZERO); if (m == NULL) { @@ -436,7 +435,6 @@ if (__predict_false((park->park_flags & PARKFLAG_DONE) || (park->park_flags & PARKFLAG_HASERROR))) { rv = park->park_preq->preq_rv; - DPRINTF(("puffs_msg_wait: park has error: %d; park=%p\n", rv, park)); mtx_unlock(&park->park_mtx); goto skipwait; } @@ -781,7 +779,7 @@ static void puffsop_suspend(struct puffs_mount *pmp) { - /* XXX: ignore */ + /* XXX ignore */ } static void ==== //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_node.c#9 (text+ko) ==== @@ -88,7 +88,6 @@ struct puffs_node_hashlist *plist; int error; - DTRACE(); KASSERT(mp != NULL, ("mp == NULL")); pmp = MPTOPUFFSMP(mp); @@ -105,11 +104,9 @@ goto bad; } - pnode = uma_zalloc(puffs_pnpool, M_WAITOK); - memset(pnode, 0, sizeof(struct puffs_node)); + pnode = uma_zalloc(puffs_pnpool, M_WAITOK | M_ZERO); error = getnewvnode("puffs", mp, &puffs_vnodeops, &vp); - DPRINTF(("puffs_getvnode: getnewvnode => %d; vp=%p mp=%p lock=%s\n", error, vp, mp, vp->v_interlock.lock_object.lo_name)); if (error) { uma_zfree(puffs_pnpool, pnode); goto bad; @@ -207,9 +204,7 @@ struct vnode *vp; int error; - DTRACE(); KASSERT(mp != NULL, ("mp == NULL")); - pmp = MPTOPUFFSMP(mp); /* userspace probably has this as a NULL op */ @@ -314,7 +309,6 @@ struct vnode *vp; int rv; - DTRACE(); /* * pmp_lock must be held if vref()'ing or vrele()'ing the * root vnode. the latter is controlled by puffs_inactive(). @@ -327,7 +321,6 @@ if (vp) { VI_LOCK(vp); mtx_unlock(&pmp->pmp_lock); - DPRINTF(("puffs_makeroot: vnode found: vp=%p mp=%p\n", vp, vp->v_mount)); vholdl(vp); vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, curthread); vdrop(vp); @@ -383,14 +376,11 @@ struct vnode *vp; int rv; - DPRINTF(("puffs_cookie2vnode lkflag=%x\n", lkflag)); - /* * Handle root in a special manner, since we want to make sure * pmp_root is properly set. */ if (ck == pmp->pmp_root_cookie) { - DPRINTF(("puffs_cookie2vnode make root lkflag=%x\n", lkflag)); if ((rv = puffs_makeroot(pmp, lkflag))) return rv; *vpp = pmp->pmp_root; @@ -415,13 +405,9 @@ VI_LOCK(vp); mtx_unlock(&pmp->pmp_lock); - DPRINTF(("puffs_cookie2vnode vget vp=%p lkflag=%x lock=%x\n", vp, lkflag, lkflag & LK_TYPE_MASK)); vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, curthread); *vpp = vp; - - ASSERT_VI_UNLOCKED(vp, "puffs_cookie2vnode"); - return 0; } @@ -480,7 +466,7 @@ mtx_lock(&pn->pn_mtx); if (--pn->pn_refcount == 0) { - DPRINTF(("puffs_releasenode: destroy puffs_node; pn_refcount=%d pn_vp=%p\n", pn->pn_refcount, pn->pn_vp)); + DPRINTF(("puffs_releasenode: destroy puffs_node pn_vp=%p\n", pn->pn_vp)); mtx_unlock(&pn->pn_mtx); mtx_destroy(&pn->pn_mtx); knlist_destroy(&pn->pn_sel.si_note); ==== //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_subr.c#6 (text+ko) ==== @@ -104,7 +104,7 @@ struct buf *bp = arg; size_t moved; - DTRACE(); + DPRINTF(("%s\n", __func__)); bp->b_error = checkerr(pmp, preq->preq_rv, __func__); if (bp->b_error == 0) { @@ -130,7 +130,7 @@ struct puffs_vnmsg_write *write_msg = (void *)preq; struct buf *bp = arg; - DTRACE(); + DPRINTF(("%s\n", __func__)); bp->b_error = checkerr(pmp, preq->preq_rv, __func__); if (bp->b_error == 0) { ==== //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_sys.h#6 (text+ko) ==== @@ -58,11 +58,9 @@ #ifdef PUFFSDEBUG extern int puffsdebug; /* puffs_subr.c */ -#define DTRACE() if (puffsdebug > 0) printf("%s\n", __func__) #define DPRINTF(x) if (puffsdebug > 0) printf x #define DPRINTF_VERBOSE(x) if (puffsdebug > 1) printf x #else -#define DTRACE() #define DPRINTF(x) #define DPRINTF_VERBOSE(x) #endif ==== //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_vfsops.c#8 (text+ko) ==== @@ -88,8 +88,6 @@ int error = 0, i; pid_t mntpid = curthread->td_proc->p_pid; - DTRACE(); - if (vfs_filteropt(mp->mnt_optnew, puffs_opts)) return EINVAL; @@ -126,7 +124,6 @@ args->pa_fhsize = MAXFIDSZ; /* sanitize file handle length */ - /* TODO increase pa_fhsize */ if (PUFFS_TOFHSIZE(args->pa_fhsize) > sizeof(struct fid)) { printf("puffs_mount: handle size %zu too large\n", args->pa_fhsize); @@ -220,7 +217,6 @@ if ((pmp->pmp_pi = putter_attach(mntpid, args->pa_fd, pmp, &puffs_putter)) == NULL) { error = ENOENT; - DPRINTF(("puffs_vfsop_mount: putter_attach attach error\n")); goto out; } @@ -241,9 +237,6 @@ mp, MPTOPUFFSMP(mp))); mp->mnt_data = pmp; - - DPRINTF(("puffs mount from: %s\n", args->pa_mntfromname)); - vfs_getnewfsid(mp); vfs_mountedfrom(mp, args->pa_mntfromname); @@ -263,7 +256,6 @@ struct puffs_mount *pmp; int error, force; - DTRACE(); error = 0; force = mntflags & MNT_FORCE; pmp = MPTOPUFFSMP(mp); @@ -359,7 +351,6 @@ int rv; - DTRACE(); rv = puffs_cookie2vnode(pmp, pmp->pmp_root_cookie, flags, 1, vpp); KASSERT(rv != PUFFS_NOSUCHCOOKIE, ("rv != PUFFS_NOSUCHCOOKIE")); @@ -390,10 +381,8 @@ * requesting statvfs from userspace would mean a deadlock. * Compensate. */ - if (pmp->pmp_status == PUFFSTAT_MOUNTING) { - DPRINTF(("puffs_vfsop_statfs: mounting\n")); + if (pmp->pmp_status == PUFFSTAT_MOUNTING) return EINPROGRESS; - } PUFFS_MSG_ALLOC(vfs, statvfs); puffs_msg_setinfo(park_statvfs, PUFFSOP_VFS, PUFFS_VFS_STATVFS, NULL); @@ -468,7 +457,6 @@ size_t argsize, fhlen; int error; - DTRACE(); if (pmp->pmp_args.pa_fhsize == 0) return EOPNOTSUPP; @@ -527,7 +515,6 @@ KASSERT(VNOVAL == VSIZENOTSET, ("VNOVAL == VSIZENOTSET")); #endif - DTRACE(); puffs_pnpool = uma_zcreate("puffpnpl", sizeof(struct puffs_node), NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_ZINIT); puffs_msgif_init(); @@ -539,7 +526,6 @@ puffs_vfsop_uninit(struct vfsconf *vfsp) { - DTRACE(); puffs_msgif_destroy(); uma_zdestroy(puffs_pnpool); ==== //depot/projects/soc2009/tatsianka_puffs/puffs/puffs_vnops.c#9 (text+ko) ==== @@ -128,7 +128,6 @@ size_t argsize, fhlen; int error, ltype; - DTRACE(); if (pmp->pmp_args.pa_fhsize == 0) return EOPNOTSUPP; @@ -199,6 +198,9 @@ isdot = cnp->cn_namelen == 1 && *cnp->cn_nameptr == '.'; + DPRINTF(("puffs_lookup: \"%s\", parent vnode %p, op: %lx\n", + cnp->cn_nameptr, dvp, cnp->cn_nameiop)); + /* * Check if someone fed it into the cache */ @@ -227,8 +229,6 @@ puffs_msg_setinfo(park_lookup, PUFFSOP_VN, PUFFS_VN_LOOKUP, VPTOPNC(dvp)); - DPRINTF(("puffs_lookup: enqueue message: %.*s\n", (int) cnp->cn_namelen, cnp->cn_nameptr)); - puffs_msg_enqueue(pmp, park_lookup); puffs_unlockvnode(dpn, &dltype); error = puffs_msg_wait2(pmp, park_lookup, dpn, NULL); @@ -327,6 +327,9 @@ int dltype; int error; + DPRINTF(("puffs_create: dvp %p, cnp: %s\n", + dvp, ap->a_cnp->cn_nameptr)); + PUFFS_MSG_ALLOC(vn, create); puffs_makecn(&create_msg->pvnr_cn, &create_msg->pvnr_cn_cred, cnp, PUFFS_USE_FULLPNBUF(pmp)); @@ -375,7 +378,6 @@ int dltype; int error; - DTRACE(); PUFFS_MSG_ALLOC(vn, mknod); puffs_makecn(&mknod_msg->pvnr_cn, &mknod_msg->pvnr_cn_cred, cnp, PUFFS_USE_FULLPNBUF(pmp)); @@ -418,6 +420,8 @@ int ltype; int error; + DPRINTF(("puffs_open: vp %p, mode 0x%x\n", vp, mode)); + if (vp->v_type == VREG && mode & FWRITE && !EXISTSOP(pmp, WRITE)) return EROFS; @@ -697,7 +701,6 @@ puffs_msg_enqueue(pmp, park_inactive); PUFFS_MSG_RELEASE(inactive); } - DPRINTF(("puffs_vnop_inactive: vp=%p\n", vp)); pnode->pn_stat &= ~PNODE_DOINACT; /* @@ -736,7 +739,6 @@ struct vnode *vp = ap->a_vp; struct puffs_mount *pmp = MPTOPUFFSMP(vp->v_mount); - DTRACE(); /* * first things first: check if someone is trying to reclaim the * root vnode. do not allow that to travel to userspace. @@ -931,12 +933,15 @@ PUFFS_VN_FSYNC, VPTOPNC(vp)); puffs_msg_enqueue(pmp, park_fsync); - puffs_unlockvnode(pn, <ype); - error = puffs_msg_wait2(pmp, park_fsync, pn, NULL); - error = checkerr(pmp, error, __func__); - + if (!dofaf) { + puffs_unlockvnode(pn, <ype); + error = puffs_msg_wait2(pmp, park_fsync, pn, NULL); + error = checkerr(pmp, error, __func__); + PUFFS_LOCKVNODE(pn, ltype, error); + } else { + error = 0; + } PUFFS_MSG_RELEASE(fsync); - PUFFS_LOCKVNODE(pn, ltype, error); return error; } @@ -1621,12 +1626,11 @@ puffs_unlockvnode(pn, <ype); error = puffs_msg_wait2(pmp, park_pathconf, pn, NULL); error = checkerr(pmp, error, __func__); - - PUFFS_MSG_RELEASE(pathconf); PUFFS_LOCKVNODE(pn, ltype, error); if (!error) *ap->a_retval = pathconf_msg->pvnr_retval; + PUFFS_MSG_RELEASE(pathconf); return error; } @@ -1680,7 +1684,6 @@ size_t tomove, moved; int error, dofaf, dobiodone; - DTRACE(); pmp = MPTOPUFFSMP(vp->v_mount); bp = ap->a_bp; error = 0; ==== //depot/projects/soc2009/tatsianka_puffs/putter/putter.c#6 (text+ko) ==== @@ -40,7 +40,6 @@ */ #include <sys/param.h> -#include <sys/types.h> #include <sys/systm.h> #include <sys/conf.h> #include <sys/fcntl.h> @@ -115,7 +114,6 @@ size_t origres, moved; int error; - DPRINTF(("putter_fop_read (%p)\n", pi)); if (pi->pi_private == PUTTER_EMBRYO || pi->pi_private == PUTTER_DEAD) { printf("putter_fop_read: private %d not inited\n", pi->pi_idx); return ENOENT; @@ -200,8 +198,6 @@ struct putter_instance *pi = dev->si_drv1; int revents; - DPRINTF(("putter_fop_poll\n")); - if (pi->pi_private == PUTTER_EMBRYO || pi->pi_private == PUTTER_DEAD) { printf("putter_fop_ioctl: putter %d not inited\n", pi->pi_idx); return ENOENT; @@ -209,7 +205,6 @@ revents = events & (POLLOUT | POLLWRNORM | POLLWRBAND); if ((events & PUTTERPOLL_EVSET) == 0) { - DPRINTF(("putter_fop_poll => %d: (events & PUTTERPOLL_EVSET) == 0\n", revents)); return revents; } @@ -219,7 +214,6 @@ else selrecord(curthread, &pi->pi_sel); - DPRINTF(("putter_fop_poll => %d\n", revents)); return revents; } @@ -321,19 +315,16 @@ struct putter_instance *pi = kn->kn_hook; int error, rv; - DPRINTF(("filt_putter: pi=%p\n", pi)); mtx_assert(&pi_mtx, MA_OWNED); error = 0; if (pi->pi_private == PUTTER_EMBRYO || pi->pi_private == PUTTER_DEAD) error = 1; if (error) { - DPRINTF(("filt_putter: error: PUTTER_EMBRYO || PUTTER_DEAD\n")); return 0; } kn->kn_data = pi->pi_pop->pop_waitcount(pi->pi_private); rv = kn->kn_data != 0; - DPRINTF(("filt_putter: rv=%d kn_data=%d\n", rv, kn->kn_data)); return rv; } @@ -381,7 +372,6 @@ switch (kn->kn_filter) { case EVFILT_READ: - DPRINTF(("putter_fop_kqfilter: read\n")); kn->kn_fop = &putter_filtops; kn->kn_hook = pi; @@ -391,11 +381,9 @@ break; case EVFILT_WRITE: - DPRINTF(("putter_fop_kqfilter: write\n")); kn->kn_fop = &putter_seltrue_filtops; break; default: - DPRINTF(("putter_fop_kqfilter: default\n")); return EINVAL; } @@ -428,8 +416,6 @@ pid = td->td_proc->p_pid; pi = dev->si_drv1; - DPRINTF(("PUTTER: open: pid=%d, pi=%p\n", pid, pi)); - if (pi != NULL) { mtx_lock(&pi_mtx); error = (pi->pi_pid == pid ? 0 : EBUSY); @@ -449,11 +435,9 @@ TAILQ_INSERT_TAIL(&putter_ilist, pi, pi_entries); - DPRINTF(("PUTTER: open: pid=%d, unit=%u\n", pi->pi_pid, pi->pi_idx)); + DPRINTF(("puttercdopen: registered embryonic pmp for pid: %d\n", + pi->pi_pid)); - DPRINTF(("puttercdopen: registered embryonic pmp for: %d/%d\n", - pi->pi_pid, pi->pi_idx)); - return (0); } @@ -472,13 +456,9 @@ { struct putter_instance *pi = NULL; - DPRINTF(("putter_attach: pid=%u, unit=%d, pprivate=%p\n", pid, unit, ppriv)); - mtx_lock(&pi_mtx); TAILQ_FOREACH(pi, &putter_ilist, pi_entries) { - DPRINTF(("putter_attach: try pid=%u, unit=%d\n", pi->pi_pid, pi->pi_idx)); - if (pi->pi_pid == pid && /* XXX_TS pi->pi_idx == unit && */ - pi->pi_private == PUTTER_EMBRYO) { + if (pi->pi_pid == pid && pi->pi_private == PUTTER_EMBRYO) { pi->pi_private = ppriv; pi->pi_pop = pop; break; @@ -511,7 +491,6 @@ putter_notify(struct putter_instance *pi) { - DPRINTF(("putter_notify: pi=%p\n", pi)); selwakeup(&pi->pi_sel); KNOTE_UNLOCKED(&pi->pi_sel.si_note, 0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908060656.n766uUTe089775>