Module Name: src
Committed By: christos
Date: Sun Aug 20 18:09:25 UTC 2023
Modified Files:
src/sys/compat/linux/common: linux_inotify.c
Log Message:
fix locking: eliminate using mutex_owned() (Theodore Preduta)
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/compat/linux/common/linux_inotify.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/compat/linux/common/linux_inotify.c
diff -u src/sys/compat/linux/common/linux_inotify.c:1.1 src/sys/compat/linux/common/linux_inotify.c:1.2
--- src/sys/compat/linux/common/linux_inotify.c:1.1 Sat Aug 19 13:57:54 2023
+++ src/sys/compat/linux/common/linux_inotify.c Sun Aug 20 14:09:25 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: linux_inotify.c,v 1.1 2023/08/19 17:57:54 christos Exp $ */
+/* $NetBSD: linux_inotify.c,v 1.2 2023/08/20 18:09:25 christos Exp $ */
/*-
* Copyright (c) 2023 The NetBSD Foundation, Inc.
@@ -29,7 +29,7 @@
* POSSIBILITY OF SUCH DAMAGE.
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: linux_inotify.c,v 1.1 2023/08/19 17:57:54 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: linux_inotify.c,v 1.2 2023/08/20 18:09:25 christos Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -135,8 +135,8 @@ static void do_kevent_to_inotify(int32_t
struct inotify_entry *, size_t *, char *);
static int kevent_to_inotify(struct inotifyfd *, int, enum vtype, uint32_t,
uint32_t, struct inotify_entry *, size_t *);
-static int inotify_readdir(file_t *, struct dirent *, int *);
-static struct inotify_dir_entries *get_inotify_dir_entries(int);
+static int inotify_readdir(file_t *, struct dirent *, int *, bool);
+static struct inotify_dir_entries *get_inotify_dir_entries(int, bool);
static int inotify_filt_attach(struct knote *);
static void inotify_filt_detach(struct knote *);
@@ -417,15 +417,15 @@ linux_sys_inotify_add_watch(struct lwp *
syscallarg(const char *) pathname;
syscallarg(uint32_t) mask;
} */
- int wd, dup_of_wd, i, error = 0;
+ int wd, i, error = 0;
file_t *fp, *wp, *cur_fp;
- struct stat wst, cur_st;
struct inotifyfd *ifd;
struct inotify_dir_entries **new_wds;
struct knote *kn, *tmpkn;
struct sys_open_args oa;
struct kevent kev;
- enum vtype wtype;
+ struct vnode *wvp;
+ namei_simple_flags_t sflags;
struct kevent_ops k_ops = {
.keo_private = NULL,
.keo_fetch_timeout = NULL,
@@ -452,38 +452,16 @@ linux_sys_inotify_add_watch(struct lwp *
mutex_enter(&ifd->ifd_lock);
- /* open a new file descriptor for the watch descriptor */
- SCARG(&oa, path) = SCARG(uap, pathname);
- SCARG(&oa, mode) = 0;
- SCARG(&oa, flags) = O_RDONLY;
if (mask & LINUX_IN_DONT_FOLLOW)
- SCARG(&oa, flags) |= O_NOFOLLOW;
- if (mask & LINUX_IN_ONLYDIR)
- SCARG(&oa, flags) |= O_DIRECTORY;
-
- error = sys_open(l, &oa, retval);
+ sflags = NSM_NOFOLLOW_TRYEMULROOT;
+ else
+ sflags = NSM_FOLLOW_TRYEMULROOT;
+ error = namei_simple_user(SCARG(uap, pathname), sflags, &wvp);
if (error != 0)
goto leave1;
- wd = *retval;
-
- wp = fd_getfile(wd);
- KASSERT(wp != NULL);
- wtype = wp->f_vnode->v_type;
- error = vn_stat(wp->f_vnode, &wst);
- fd_putfile(wd);
- if (error != 0)
- goto leave1;
-
- /* translate the flags */
- memset(&kev, 0, sizeof(kev));
- EV_SET(&kev, wd, inotify_filtid, EV_ADD|EV_ENABLE,
- NOTE_DELETE|NOTE_REVOKE, 0, ifd);
- if (mask & LINUX_IN_ONESHOT)
- kev.flags |= EV_ONESHOT;
- kev.fflags |= inotify_mask_to_kevent_fflags(mask, wtype);
/* Check to see if we already have a descriptor to wd's file. */
- dup_of_wd = -1;
+ wd = -1;
for (i = 0; i < ifd->ifd_nwds; i++) {
if (ifd->ifd_wds[i] != NULL) {
cur_fp = fd_getfile(i);
@@ -498,24 +476,47 @@ linux_sys_inotify_add_watch(struct lwp *
"with a non-vnode\n", __func__, i));
error = EBADF;
}
- if (error == 0)
- error = vn_stat(cur_fp->f_vnode, &cur_st);
+ if (error == 0 && cur_fp->f_vnode == wvp)
+ wd = i;
fd_putfile(i);
if (error != 0)
goto leave1;
- if (wst.st_ino == cur_st.st_ino) {
- dup_of_wd = i;
+ if (wd != -1)
break;
- }
}
}
- if (dup_of_wd != -1) {
+ if (wd == -1) {
/*
- * If we do not have a descriptor to wd's file, we need to add
- * a knote.
+ * If we do not have a descriptor to wd's file, we
+ * need to open the watch descriptor.
*/
+ SCARG(&oa, path) = SCARG(uap, pathname);
+ SCARG(&oa, mode) = 0;
+ SCARG(&oa, flags) = O_RDONLY;
+ if (mask & LINUX_IN_DONT_FOLLOW)
+ SCARG(&oa, flags) |= O_NOFOLLOW;
+ if (mask & LINUX_IN_ONLYDIR)
+ SCARG(&oa, flags) |= O_DIRECTORY;
+
+ error = sys_open(l, &oa, retval);
+ if (error != 0)
+ goto leave1;
+ wd = *retval;
+ wp = fd_getfile(wd);
+ KASSERT(wp != NULL);
+ KASSERT(wp->f_type == DTYPE_VNODE);
+
+ /* translate the flags */
+ memset(&kev, 0, sizeof(kev));
+ EV_SET(&kev, wd, inotify_filtid, EV_ADD|EV_ENABLE,
+ NOTE_DELETE|NOTE_REVOKE, 0, ifd);
+ if (mask & LINUX_IN_ONESHOT)
+ kev.flags |= EV_ONESHOT;
+ kev.fflags |= inotify_mask_to_kevent_fflags(mask,
+ wp->f_vnode->v_type);
+
error = kevent1(retval, ifd->ifd_kqfd, &kev, 1, NULL, 0, NULL,
&k_ops);
if (error != 0) {
@@ -539,30 +540,34 @@ linux_sys_inotify_add_watch(struct lwp *
ifd->ifd_nwds = wd+1;
}
- ifd->ifd_wds[wd] = get_inotify_dir_entries(wd);
+ ifd->ifd_wds[wd] = get_inotify_dir_entries(wd, true);
}
} else {
/*
* If we do have a descriptor to wd's file, try to edit
* the relevant knote.
*/
-
- /* We do not need wd anymore. */
- fd_getfile(wd);
- fd_close(wd);
-
if (mask & LINUX_IN_MASK_CREATE) {
error = EEXIST;
goto leave1;
}
- wp = fd_getfile(dup_of_wd);
+ wp = fd_getfile(wd);
if (wp == NULL) {
DPRINTF(("%s: wd=%d was closed externally "
- "(race, probably)\n", __func__, dup_of_wd));
+ "(race, probably)\n", __func__, wd));
error = EBADF;
goto leave1;
}
+ if (wp->f_type != DTYPE_VNODE) {
+ DPRINTF(("%s: wd=%d was replace with a non-vnode "
+ "(race, probably)\n", __func__, wd));
+ error = EBADF;
+ goto leave2;
+ }
+
+ kev.fflags = NOTE_DELETE | NOTE_REVOKE
+ | inotify_mask_to_kevent_fflags(mask, wp->f_vnode->v_type);
mutex_enter(wp->f_vnode->v_interlock);
@@ -588,9 +593,13 @@ linux_sys_inotify_add_watch(struct lwp *
}
mutex_exit(wp->f_vnode->v_interlock);
- fd_putfile(dup_of_wd);
+
+ /* Success! */
+ *retval = wd;
}
+leave2:
+ fd_putfile(wd);
leave1:
mutex_exit(&ifd->ifd_lock);
leave0:
@@ -748,11 +757,11 @@ do_kevent_to_inotify(int32_t wd, uint32_
}
/*
- * Like vn_readdir(), but with vnode locking that depends on if we already have
- * v_interlock (to avoid double locking in some situations).
+ * Like vn_readdir(), but with vnode locking only if needs_lock is
+ * true (to avoid double locking in some situations).
*/
static int
-inotify_readdir(file_t *fp, struct dirent *dep, int *done)
+inotify_readdir(file_t *fp, struct dirent *dep, int *done, bool needs_lock)
{
struct vnode *vp;
struct iovec iov;
@@ -777,10 +786,10 @@ inotify_readdir(file_t *fp, struct diren
mutex_exit(&fp->f_lock);
/* XXX: should pass whether to lock or not */
- if (!mutex_owned(vp->v_interlock))
+ if (needs_lock)
vn_lock(vp, LK_SHARED | LK_RETRY);
error = VOP_READDIR(vp, &uio, fp->f_cred, &eofflag, NULL, NULL);
- if (!mutex_owned(vp->v_interlock))
+ if (needs_lock)
VOP_UNLOCK(vp);
mutex_enter(&fp->f_lock);
@@ -794,10 +803,11 @@ inotify_readdir(file_t *fp, struct diren
/*
* Create (and allocate) an appropriate inotify_dir_entries struct for wd to be
* used on ifd_wds of inotifyfd. If the entries on a directory fail to be read,
- * NULL is returned.
+ * NULL is returned. needs_lock indicates if the vnode's lock is not already
+ * owned.
*/
static struct inotify_dir_entries *
-get_inotify_dir_entries(int wd)
+get_inotify_dir_entries(int wd, bool needs_lock)
{
struct dirent de;
struct dirent *currdep;
@@ -823,7 +833,7 @@ get_inotify_dir_entries(int wd)
mutex_exit(&wp->f_lock);
decount = 0;
for (;;) {
- error = inotify_readdir(wp, &de, &done);
+ error = inotify_readdir(wp, &de, &done, needs_lock);
if (error != 0)
goto leave;
if (done == 0)
@@ -843,7 +853,7 @@ get_inotify_dir_entries(int wd)
wp->f_offset = 0;
mutex_exit(&wp->f_lock);
for (i = 0; i < decount;) {
- error = inotify_readdir(wp, &de, &done);
+ error = inotify_readdir(wp, &de, &done, needs_lock);
if (error != 0 || done == 0) {
kmem_free(idep, INOTIFY_DIR_ENTRIES_SIZE(decount));
idep = NULL;
@@ -886,7 +896,7 @@ handle_write(struct inotifyfd *ifd, int
old_idep = ifd->ifd_wds[wd];
KASSERT(old_idep != NULL);
- new_idep = get_inotify_dir_entries(wd);
+ new_idep = get_inotify_dir_entries(wd, false);
if (new_idep == NULL) {
DPRINTF(("%s: directory for wd=%d could not be read\n",
__func__, wd));
@@ -1049,17 +1059,6 @@ inotify_filt_event(struct knote *kn, lon
return 0;
/*
- * Because we use kqueue() and file descriptors underneath,
- * functions like inotify_add_watch can actually trigger
- * events (ie. we're watching for LINUX_IN_OPEN). In all
- * cases where this could happen, we must already own
- * ifd->ifd_lock, so we can just drop these events.
- */
- /* XXX: why do we need this here? */
- if (mutex_owned(&ifd->ifd_lock))
- return 0;
-
- /*
* If we don't care about the NOTEs in hint, we don't generate
* any events.
*/
@@ -1068,6 +1067,7 @@ inotify_filt_event(struct knote *kn, lon
return 0;
KASSERT(mutex_owned(vp->v_interlock));
+ KASSERT(!mutex_owned(&ifd->ifd_lock));
mutex_enter(&ifd->ifd_qlock);
