From f176a0a4cd61cbd708a7f25dc30d221f4d5902ba Mon Sep 17 00:00:00 2001 From: Bryan Cantrill Date: Mon, 2 Mar 2015 21:52:48 +0000 Subject: [PATCH] 8270 dnlc_reverse_lookup() is unsafe at any speed Reviewed by: Patrick Mooney Reviewed by: Andy Stormont Reviewed by: Yuri Pankov Reviewed by: Toomas Soome Approved by: Richard Lowe --- usr/src/uts/common/fs/dnlc.c | 59 +++++---------------------------- usr/src/uts/common/fs/lookup.c | 75 ++++-------------------------------------- usr/src/uts/common/sys/dnlc.h | 1 - 3 files changed, 14 insertions(+), 121 deletions(-) diff --git a/usr/src/uts/common/fs/dnlc.c b/usr/src/uts/common/fs/dnlc.c index 25112ac4aa..0ec57ff7f7 100644 --- a/usr/src/uts/common/fs/dnlc.c +++ b/usr/src/uts/common/fs/dnlc.c @@ -20,6 +20,7 @@ */ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, Joyent, Inc. * Copyright (c) 2017 by Delphix. All rights reserved. */ @@ -67,12 +68,12 @@ /* * We want to be able to identify files that are referenced only by the DNLC. * When adding a reference from the DNLC, call VN_HOLD_DNLC instead of VN_HOLD, - * since multiple DNLC references should only be counted once in v_count. This - * file contains only two(2) calls to VN_HOLD, renamed VN_HOLD_CALLER in the - * hope that no one will mistakenly add a VN_HOLD to this file. (Unfortunately - * it is not possible to #undef VN_HOLD and retain VN_HOLD_CALLER. Ideally a - * Makefile rule would grep uncommented C tokens to check that VN_HOLD is - * referenced only once in this file, to define VN_HOLD_CALLER.) + * since multiple DNLC references should only be counted once in v_count. The + * VN_HOLD macro itself is aliased to VN_HOLD_CALLER in this file to help + * differentiate the behaviors. (Unfortunately it is not possible to #undef + * VN_HOLD and retain VN_HOLD_CALLER. Ideally a Makefile rule would grep + * uncommented C tokens to check that VN_HOLD is referenced only once in this + * file, to define VN_HOLD_CALLER.) */ #define VN_HOLD_CALLER VN_HOLD #define VN_HOLD_DNLC(vp) { \ @@ -634,7 +635,7 @@ dnlc_lookup(vnode_t *dp, const char *name) * put a hold on it. */ vp = ncp->vp; - VN_HOLD_CALLER(vp); /* VN_HOLD 1 of 2 in this file */ + VN_HOLD_CALLER(vp); mutex_exit(&hp->hash_lock); ncstats.hits++; ncs.ncs_hits.value.ui64++; @@ -922,50 +923,6 @@ dnlc_fs_purge1(vnodeops_t *vop) } /* - * Perform a reverse lookup in the DNLC. This will find the first occurrence of - * the vnode. If successful, it will return the vnode of the parent, and the - * name of the entry in the given buffer. If it cannot be found, or the buffer - * is too small, then it will return NULL. Note that this is a highly - * inefficient function, since the DNLC is constructed solely for forward - * lookups. - */ -vnode_t * -dnlc_reverse_lookup(vnode_t *vp, char *buf, size_t buflen) -{ - nc_hash_t *nch; - ncache_t *ncp; - vnode_t *pvp; - - if (!doingcache) - return (NULL); - - for (nch = nc_hash; nch < &nc_hash[nc_hashsz]; nch++) { - mutex_enter(&nch->hash_lock); - ncp = nch->hash_next; - while (ncp != (ncache_t *)nch) { - /* - * We ignore '..' entries since it can create - * confusion and infinite loops. - */ - if (ncp->vp == vp && !(ncp->namlen == 2 && - 0 == bcmp(ncp->name, "..", 2)) && - ncp->namlen < buflen) { - bcopy(ncp->name, buf, ncp->namlen); - buf[ncp->namlen] = '\0'; - pvp = ncp->dp; - /* VN_HOLD 2 of 2 in this file */ - VN_HOLD_CALLER(pvp); - mutex_exit(&nch->hash_lock); - return (pvp); - } - ncp = ncp->hash_next; - } - mutex_exit(&nch->hash_lock); - } - - return (NULL); -} -/* * Utility routine to search for a cache entry. Return the * ncache entry if found, NULL otherwise. */ diff --git a/usr/src/uts/common/fs/lookup.c b/usr/src/uts/common/fs/lookup.c index 55ffb94805..da0eae21e5 100644 --- a/usr/src/uts/common/fs/lookup.c +++ b/usr/src/uts/common/fs/lookup.c @@ -22,6 +22,7 @@ /* * Copyright 2015 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 1988, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, Joyent, Inc. */ /* Copyright (c) 1983, 1984, 1985, 1986, 1987, 1988, 1989 AT&T */ @@ -1166,38 +1167,6 @@ dirtopath(vnode_t *vrootp, vnode_t *vp, char *buf, size_t buflen, int flags, } /* - * Try to obtain the path component from dnlc cache - * before searching through the directory. - */ - if ((cmpvp = dnlc_reverse_lookup(vp, dbuf, dlen)) != NULL) { - /* - * If we got parent vnode as a result, - * then the answered path is correct. - */ - if (VN_CMP(cmpvp, pvp)) { - VN_RELE(cmpvp); - complen = strlen(dbuf); - bufloc -= complen; - if (bufloc <= buf) { - err = ENAMETOOLONG; - goto out; - } - bcopy(dbuf, bufloc, complen); - - /* Prepend a slash to the current path */ - *--bufloc = '/'; - - /* And continue with the next component */ - VN_RELE(vp); - vp = pvp; - pvp = NULL; - continue; - } else { - VN_RELE(cmpvp); - } - } - - /* * Search the parent directory for the entry corresponding to * this vnode. */ @@ -1269,10 +1238,9 @@ vnodetopath_common(vnode_t *vrootp, vnode_t *vp, char *buf, size_t buflen, cred_t *cr, int flags) { pathname_t pn, rpn; - int ret, len; - vnode_t *compvp, *pvp, *realvp; + int ret; + vnode_t *compvp, *realvp; proc_t *p = curproc; - char path[MAXNAMELEN]; int doclose = 0; /* @@ -1409,41 +1377,10 @@ notcached: pn_free(&pn); if (vp->v_type != VDIR) { - /* - * If we don't have a directory, try to find it in the dnlc via - * reverse lookup. Once this is found, we can use the regular - * directory search to find the full path. - */ - if ((pvp = dnlc_reverse_lookup(vp, path, MAXNAMELEN)) != NULL) { - /* - * Check if we have read privilege so, that - * we can lookup the path in the directory - */ - ret = 0; - if ((flags & LOOKUP_CHECKREAD)) { - ret = VOP_ACCESS(pvp, VREAD, 0, cr, NULL); - } - if (ret == 0) { - ret = dirtopath(vrootp, pvp, buf, buflen, - flags, cr); - } - if (ret == 0) { - len = strlen(buf); - if (len + strlen(path) + 1 >= buflen) { - ret = ENAMETOOLONG; - } else { - if (buf[len - 1] != '/') - buf[len++] = '/'; - bcopy(path, buf + len, - strlen(path) + 1); - } - } - - VN_RELE(pvp); - } else - ret = ENOENT; - } else + ret = ENOENT; + } else { ret = dirtopath(vrootp, vp, buf, buflen, flags, cr); + } VN_RELE(vrootp); if (doclose) { diff --git a/usr/src/uts/common/sys/dnlc.h b/usr/src/uts/common/sys/dnlc.h index 070506ee31..26f1f1dc0c 100644 --- a/usr/src/uts/common/sys/dnlc.h +++ b/usr/src/uts/common/sys/dnlc.h @@ -187,7 +187,6 @@ void dnlc_purge_vp(vnode_t *); int dnlc_purge_vfsp(vfs_t *, int); void dnlc_remove(vnode_t *, const char *); int dnlc_fs_purge1(struct vnodeops *); -vnode_t *dnlc_reverse_lookup(vnode_t *, char *, size_t); void dnlc_reduce_cache(void *); #endif /* defined(_KERNEL) */ -- 2.11.4.GIT