From e10a268edd841334ce53dd5a87213dbd7e3b4baa Mon Sep 17 00:00:00 2001 From: mvs Date: Sat, 16 Dec 2023 22:17:08 +0000 Subject: [PATCH] Make `fuse_rd_filtops' mpsafe. Introduce `fd_lock' rwlock(9) and use it for `fd_fbufs_in' fuse buffers queue and `fd_rklist' knotes list protection. Tested by Rafael Sadowski. Discussed with and ok from bluhm --- sys/miscfs/fuse/fuse_device.c | 84 ++++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 16 deletions(-) diff --git a/sys/miscfs/fuse/fuse_device.c b/sys/miscfs/fuse/fuse_device.c index cd6c627b195..b81fcea9bb2 100644 --- a/sys/miscfs/fuse/fuse_device.c +++ b/sys/miscfs/fuse/fuse_device.c @@ -1,4 +1,4 @@ -/* $OpenBSD: fuse_device.c,v 1.39 2023/09/08 20:00:28 mvs Exp $ */ +/* $OpenBSD: fuse_device.c,v 1.40 2023/12/16 22:17:08 mvs Exp $ */ /* * Copyright (c) 2012-2013 Sylvestre Gallon * @@ -19,13 +19,14 @@ #include #include #include +#include #include #include +#include #include #include #include #include -#include #include "fusefs_node.h" #include "fusefs.h" @@ -36,18 +37,24 @@ #define DPRINTF(fmt, arg...) #endif +/* + * Locks used to protect struct members and global data + * l fd_lock + */ + SIMPLEQ_HEAD(fusebuf_head, fusebuf); struct fuse_d { + struct rwlock fd_lock; struct fusefs_mnt *fd_fmp; int fd_unit; /*fusebufs queues*/ - struct fusebuf_head fd_fbufs_in; + struct fusebuf_head fd_fbufs_in; /* [l] */ struct fusebuf_head fd_fbufs_wait; /* kq fields */ - struct selinfo fd_rsel; + struct klist fd_rklist; /* [l] */ LIST_ENTRY(fuse_d) fd_list; }; @@ -67,12 +74,16 @@ int fusewrite(dev_t, struct uio *, int); int fusekqfilter(dev_t dev, struct knote *kn); int filt_fuse_read(struct knote *, long); void filt_fuse_rdetach(struct knote *); +int filt_fuse_modify(struct kevent *, struct knote *); +int filt_fuse_process(struct knote *, struct kevent *); static const struct filterops fuse_rd_filtops = { - .f_flags = FILTEROP_ISFD, + .f_flags = FILTEROP_ISFD | FILTEROP_MPSAFE, .f_attach = NULL, .f_detach = filt_fuse_rdetach, .f_event = filt_fuse_read, + .f_modify = filt_fuse_modify, + .f_process = filt_fuse_process, }; #ifdef FUSE_DEBUG @@ -142,6 +153,7 @@ fuse_device_cleanup(dev_t dev) /* clear FIFO IN */ lprev = NULL; + rw_enter_write(&fd->fd_lock); SIMPLEQ_FOREACH_SAFE(f, &fd->fd_fbufs_in, fb_next, ftmp) { DPRINTF("cleanup unprocessed msg in sc_fbufs_in\n"); if (lprev == NULL) @@ -155,6 +167,7 @@ fuse_device_cleanup(dev_t dev) wakeup(f); lprev = f; } + rw_exit_write(&fd->fd_lock); /* clear FIFO WAIT*/ lprev = NULL; @@ -182,9 +195,11 @@ fuse_device_queue_fbuf(dev_t dev, struct fusebuf *fbuf) if (fd == NULL) return; + rw_enter_write(&fd->fd_lock); SIMPLEQ_INSERT_TAIL(&fd->fd_fbufs_in, fbuf, fb_next); + knote_locked(&fd->fd_rklist, 0); + rw_exit_write(&fd->fd_lock); stat_fbufs_in++; - selwakeup(&fd->fd_rsel); } void @@ -221,6 +236,9 @@ fuseopen(dev_t dev, int flags, int fmt, struct proc * p) fd->fd_unit = unit; SIMPLEQ_INIT(&fd->fd_fbufs_in); SIMPLEQ_INIT(&fd->fd_fbufs_wait); + rw_init(&fd->fd_lock, "fusedlk"); + klist_init_rwlock(&fd->fd_rklist, &fd->fd_lock); + LIST_INSERT_HEAD(&fuse_d_list, fd, fd_list); stat_opened_fusedev++; @@ -278,6 +296,7 @@ fuseioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) ioexch = (struct fb_ioctl_xch *)addr; /* Looking for uuid in fd_fbufs_in */ + rw_enter_write(&fd->fd_lock); SIMPLEQ_FOREACH(fbuf, &fd->fd_fbufs_in, fb_next) { if (fbuf->fb_uuid == ioexch->fbxch_uuid) break; @@ -285,6 +304,7 @@ fuseioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) lastfbuf = fbuf; } if (fbuf == NULL) { + rw_exit_write(&fd->fd_lock); printf("fuse: Cannot find fusebuf\n"); return (EINVAL); } @@ -295,6 +315,8 @@ fuseioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) else SIMPLEQ_REMOVE_AFTER(&fd->fd_fbufs_in, lastfbuf, fb_next); + rw_exit_write(&fd->fd_lock); + stat_fbufs_in--; /* Do not handle fbufs with bad len */ @@ -385,22 +407,23 @@ fuseread(dev_t dev, struct uio *uio, int ioflag) void *tmpaddr; int error = 0; + /* We get the whole fusebuf or nothing */ + if (uio->uio_resid != FUSEBUFSIZE) + return (EINVAL); + fd = fuse_lookup(minor(dev)); if (fd == NULL) return (ENXIO); + rw_enter_write(&fd->fd_lock); + if (SIMPLEQ_EMPTY(&fd->fd_fbufs_in)) { if (ioflag & O_NONBLOCK) - return (EAGAIN); - + error = EAGAIN; goto end; } fbuf = SIMPLEQ_FIRST(&fd->fd_fbufs_in); - /* We get the whole fusebuf or nothing */ - if (uio->uio_resid != FUSEBUFSIZE) - return (EINVAL); - /* Do not send kernel pointers */ memcpy(&hdr.fh_next, &fbuf->fb_next, sizeof(fbuf->fb_next)); memset(&fbuf->fb_next, 0, sizeof(fbuf->fb_next)); @@ -426,6 +449,7 @@ fuseread(dev_t dev, struct uio *uio, int ioflag) } end: + rw_exit_write(&fd->fd_lock); return (error); } @@ -519,7 +543,7 @@ fusekqfilter(dev_t dev, struct knote *kn) switch (kn->kn_filter) { case EVFILT_READ: - klist = &fd->fd_rsel.si_note; + klist = &fd->fd_rklist; kn->kn_fop = &fuse_rd_filtops; break; case EVFILT_WRITE: @@ -530,7 +554,7 @@ fusekqfilter(dev_t dev, struct knote *kn) kn->kn_hook = fd; - klist_insert_locked(klist, kn); + klist_insert(klist, kn); return (0); } @@ -539,9 +563,9 @@ void filt_fuse_rdetach(struct knote *kn) { struct fuse_d *fd = kn->kn_hook; - struct klist *klist = &fd->fd_rsel.si_note; + struct klist *klist = &fd->fd_rklist; - klist_remove_locked(klist, kn); + klist_remove(klist, kn); } int @@ -550,8 +574,36 @@ filt_fuse_read(struct knote *kn, long hint) struct fuse_d *fd = kn->kn_hook; int event = 0; + rw_assert_wrlock(&fd->fd_lock); + if (!SIMPLEQ_EMPTY(&fd->fd_fbufs_in)) event = 1; return (event); } + +int +filt_fuse_modify(struct kevent *kev, struct knote *kn) +{ + struct fuse_d *fd = kn->kn_hook; + int active; + + rw_enter_write(&fd->fd_lock); + active = knote_modify(kev, kn); + rw_exit_write(&fd->fd_lock); + + return (active); +} + +int +filt_fuse_process(struct knote *kn, struct kevent *kev) +{ + struct fuse_d *fd = kn->kn_hook; + int active; + + rw_enter_write(&fd->fd_lock); + active = knote_process(kn, kev); + rw_exit_write(&fd->fd_lock); + + return (active); +} -- 2.20.1