https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=293382
--- Comment #24 from Paul <[email protected]> --- Hi, Thanks! There was a small issue: NULL can actually be passed to `knote_free()`. So we have adapted the patch a bit. Also, we've never discarded all previous patches. So, to be of the same page, here's the final version that we're testing now, on the HEAD: diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c index e8e670d39d09..bbb09789e41f 100644 --- a/sys/kern/kern_event.c +++ b/sys/kern/kern_event.c @@ -28,7 +28,6 @@ * SUCH DAMAGE. */ -#include <sys/cdefs.h> #include "opt_ktrace.h" #include "opt_kqueue.h" @@ -230,6 +229,13 @@ static const struct filterops user_filtops = { .f_copy = knote_triv_copy, }; +#include <sys/stack.h> +struct eknote { + struct knote k; + struct knote c; + struct stack s; +}; + static uma_zone_t knote_zone; static unsigned int __exclusive_cache_line kq_ncallouts; static unsigned int kq_calloutmax = 4 * 1024; @@ -1803,6 +1809,19 @@ kqueue_register(struct kqueue *kq, struct kevent *kev, struct thread *td, error = ENOMEM; goto done; } + + /* + * Now that the kqueue is locked, make sure the fd + * didn't change out from under us. + */ + if (fops->f_isfd && + fget_noref_unlocked(td->td_proc->p_fd, + kev->ident) != fp) { + KQ_UNLOCK(kq); + tkn = kn; + error = EBADF; + goto done; + } kn->kn_fp = fp; kn->kn_kq = kq; kn->kn_fop = fops; @@ -2830,19 +2849,21 @@ knote_fdclose(struct thread *td, int fd) * We shouldn't have to worry about new kevents appearing on fd * since filedesc is locked. */ +again: TAILQ_FOREACH(kq, &fdp->fd_kqlist, kq_list) { KQ_LOCK(kq); -again: influx = 0; while (kq->kq_knlistsize > fd && (kn = SLIST_FIRST(&kq->kq_knlist[fd])) != NULL) { + MPASS(kn->kn_kq == kq); if (kn_in_flux(kn)) { /* someone else might be waiting on our knote */ if (influx) wakeup(kq); kq->kq_state |= KQ_FLUXWAIT; - msleep(kq, &kq->kq_lock, PSOCK, "kqflxwt", 0); + msleep(kq, &kq->kq_lock, PSOCK | PDROP, + "kqflxwt", 0); goto again; } kn_enter_flux(kn); @@ -2961,8 +2982,8 @@ static void knote_init(void *dummy __unused) { - knote_zone = uma_zcreate("KNOTE", sizeof(struct knote), NULL, NULL, - NULL, NULL, UMA_ALIGN_PTR, 0); + knote_zone = uma_zcreate("KNOTE", sizeof(struct eknote), NULL, NULL, + NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE); ast_register(TDA_KQUEUE, ASTR_ASTF_REQUIRED, 0, ast_kqueue); prison0.pr_klist = knlist_alloc(&prison0.pr_mtx); } @@ -2971,15 +2992,24 @@ SYSINIT(knote, SI_SUB_PSEUDO, SI_ORDER_ANY, knote_init, NULL); static struct knote * knote_alloc(int mflag) { + struct eknote *e; - return (uma_zalloc(knote_zone, mflag | M_ZERO)); + e = uma_zalloc(knote_zone, mflag | M_ZERO); + return (&e->k); } static void knote_free(struct knote *kn) { + if (kn) { + struct eknote *e; - uma_zfree(knote_zone, kn); + e = __containerof(kn, struct eknote, k); + e->c = e->k; + stack_save(&e->s); + memset(&e->k, 0xdeadc0de, sizeof(e->k)); + uma_zfree(knote_zone, e); + } } /* diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h index 4817855443af..9ea5239afae7 100644 --- a/sys/sys/filedesc.h +++ b/sys/sys/filedesc.h @@ -212,6 +212,8 @@ struct filedesc_to_leader { #ifdef _KERNEL +#include <machine/atomic.h> + /* Operation types for kern_dup(). */ enum { FDDUP_NORMAL, /* dup() behavior. */ @@ -303,6 +305,21 @@ int fget_only_user(struct filedesc *fdp, int fd, MPASS(refcount_load(&fp->f_count) > 0); \ }) +/* +* Look up a file description without requiring a lock. In general the result + * may be immediately invalidated after the function returns, the caller must + * handle this. + */ +static inline struct file * +fget_noref_unlocked(struct filedesc *fdp, int fd) +{ + if (__predict_false( + (u_int)fd >= (u_int)atomic_load_int(&fdp->fd_nfiles))) + return (NULL); + + return (atomic_load_ptr(&fdp->fd_ofiles[fd].fde_file)); +} + /* Requires a FILEDESC_{S,X}LOCK held and returns without a ref. */ static __inline struct file * fget_noref(struct filedesc *fdp, int fd) -- You are receiving this mail because: You are the assignee for the bug.
