From owner-freebsd-bugs@FreeBSD.ORG Thu Dec 20 23:20:02 2007 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DBB7A16A418 for ; Thu, 20 Dec 2007 23:20:01 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id BBDAC13C447 for ; Thu, 20 Dec 2007 23:20:01 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.2/8.14.2) with ESMTP id lBKNK1oV082563 for ; Thu, 20 Dec 2007 23:20:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.2/8.14.1/Submit) id lBKNK1Ea082562; Thu, 20 Dec 2007 23:20:01 GMT (envelope-from gnats) Resent-Date: Thu, 20 Dec 2007 23:20:01 GMT Resent-Message-Id: <200712202320.lBKNK1Ea082562@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, MOROHOSHI Akihiko Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A57ED16A46C for ; Thu, 20 Dec 2007 23:17:21 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 9759F13C4EA for ; Thu, 20 Dec 2007 23:17:21 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.2/8.14.2) with ESMTP id lBKNGwjD016471 for ; Thu, 20 Dec 2007 23:16:58 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.2/8.14.1/Submit) id lBKNGwXl016470; Thu, 20 Dec 2007 23:16:58 GMT (envelope-from nobody) Message-Id: <200712202316.lBKNGwXl016470@www.freebsd.org> Date: Thu, 20 Dec 2007 23:16:58 GMT From: MOROHOSHI Akihiko To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: kern/118911: kevent can cause the system to crash if aio is enabled. X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Dec 2007 23:20:02 -0000 >Number: 118911 >Category: kern >Synopsis: kevent can cause the system to crash if aio is enabled. >Confidential: no >Severity: serious >Priority: high >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Dec 20 23:20:00 UTC 2007 >Closed-Date: >Last-Modified: >Originator: MOROHOSHI Akihiko >Release: 7.0-BETA4 >Organization: >Environment: System: FreeBSD 7.0-BETA4 FreeBSD 7.0-BETA4 #0: Thu Dec 20 18:20:42 JST 2007 root@:/usr/obj/usr/src/sys/SAZANKA i386 >Description: kevent(2) modifies an existing event when called with EV_ADD: kqueue_register() overwrites kn_sfflags, kn_sdata and kn_kevent.udata in a knote with user-supplied values in that case. But filt_aio{,attach,detach}() use kn_sdata to keep kernel's internal data; therefore calling kevent(2) with EVFILT_AIO and EV_ADD can make the kernel crash. I found this bug in 6.2-STABLE, 7.0-BETA4 and 6.3-RC1. Other versions also seem to be affected. >How-To-Repeat: 1. Save the following program as aio-kevent.c. 2. gcc -Wall -g -o aio-kevent aio-kevent.c 3. kldload aio 4. ./aio-kevent < aio-kevent.c ===== aio-kevent.c ===== #include #include #include #include #include #include #include #include #include #include #include #include #include #if defined(__FreeBSD__) && __FreeBSD__ < 7 #define sival_ptr sigval_ptr #endif #define BUFSIZE 1024 static int kq; void test(int fd) { struct aiocb *aiocb = malloc(sizeof(*aiocb)); char *buf = malloc(BUFSIZE); struct kevent kev; int r; bzero(aiocb, sizeof(*aiocb)); aiocb->aio_fildes = fd; aiocb->aio_offset = 0; aiocb->aio_buf = buf; aiocb->aio_nbytes = BUFSIZE; aiocb->aio_sigevent.sigev_notify = SIGEV_KEVENT; aiocb->aio_sigevent.sigev_notify_kqueue = kq; aiocb->aio_sigevent.sigev_value.sival_ptr = aiocb; again: if ((r = aio_read(aiocb)) < 0) { if (errno == EAGAIN) goto again; perror("aio_read"); exit(1); } /* This is OK: */ /* EV_SET(&kev, (uintptr_t)aiocb, EVFILT_AIO, EV_ENABLE, 0, 0, NULL); */ /* This makes FreeBSD crash: */ EV_SET(&kev, (uintptr_t)aiocb, EVFILT_AIO, EV_ADD, 0, 0, NULL); if ((r = kevent(kq, &kev, 1, NULL, 0, NULL)) < 0) { perror("kevent"); exit(1); } } int main(int argc, char **argv) { if ((kq = kqueue()) < 0) { perror("kqueue"); exit(1); } test(STDIN_FILENO); return 0; } >Fix: The following patch seems to fix the problem in 7.0-BETA4. I assume that kn->kn_fp is not used by EVFILT_AIO or EVFILT_LIO. --- sys/kern/vfs_aio.c.orig 2007-08-20 20:53:26.000000000 +0900 +++ sys/kern/vfs_aio.c 2007-12-21 05:43:53.000000000 +0900 @@ -2251,6 +2251,7 @@ filt_aioattach(struct knote *kn) */ if ((kn->kn_flags & EV_FLAG1) == 0) return (EPERM); + kn->kn_ptr.p_aio = aiocbe; kn->kn_flags &= ~EV_FLAG1; knlist_add(&aiocbe->klist, kn, 0); @@ -2262,7 +2263,7 @@ filt_aioattach(struct knote *kn) static void filt_aiodetach(struct knote *kn) { - struct aiocblist *aiocbe = (struct aiocblist *)kn->kn_sdata; + struct aiocblist *aiocbe = kn->kn_ptr.p_aio; if (!knlist_empty(&aiocbe->klist)) knlist_remove(&aiocbe->klist, kn, 0); @@ -2273,7 +2274,7 @@ filt_aiodetach(struct knote *kn) static int filt_aio(struct knote *kn, long hint) { - struct aiocblist *aiocbe = (struct aiocblist *)kn->kn_sdata; + struct aiocblist *aiocbe = kn->kn_ptr.p_aio; kn->kn_data = aiocbe->uaiocb._aiocb_private.error; if (aiocbe->jobstate != JOBST_JOBFINISHED) @@ -2295,6 +2296,7 @@ filt_lioattach(struct knote *kn) */ if ((kn->kn_flags & EV_FLAG1) == 0) return (EPERM); + kn->kn_ptr.p_lio = lj; kn->kn_flags &= ~EV_FLAG1; knlist_add(&lj->klist, kn, 0); @@ -2306,7 +2308,7 @@ filt_lioattach(struct knote *kn) static void filt_liodetach(struct knote *kn) { - struct aioliojob * lj = (struct aioliojob *)kn->kn_sdata; + struct aioliojob * lj = kn->kn_ptr.p_lio; if (!knlist_empty(&lj->klist)) knlist_remove(&lj->klist, kn, 0); @@ -2317,7 +2319,7 @@ filt_liodetach(struct knote *kn) static int filt_lio(struct knote *kn, long hint) { - struct aioliojob * lj = (struct aioliojob *)kn->kn_sdata; + struct aioliojob * lj = kn->kn_ptr.p_lio; return (lj->lioj_flags & LIOJ_KEVENT_POSTED); } --- sys/sys/event.h.orig 2006-09-24 13:47:47.000000000 +0900 +++ sys/sys/event.h 2007-12-21 05:42:02.000000000 +0900 @@ -181,6 +181,8 @@ struct knote { union { struct file *p_fp; /* file data pointer */ struct proc *p_proc; /* proc pointer */ + struct aiocblist *p_aio; /* AIO job pointer */ + struct aioliojob *p_lio; /* LIO job pointer */ } kn_ptr; struct filterops *kn_fop; void *kn_hook; The same fix against 6.3-RC1: --- sys/kern/vfs_aio.c.orig 2006-09-09 10:30:11.000000000 +0900 +++ sys/kern/vfs_aio.c 2007-12-21 06:05:49.000000000 +0900 @@ -2257,6 +2257,7 @@ filt_aioattach(struct knote *kn) */ if ((kn->kn_flags & EV_FLAG1) == 0) return (EPERM); + kn->kn_ptr.p_aio = aiocbe; kn->kn_flags &= ~EV_FLAG1; knlist_add(&aiocbe->klist, kn, 0); @@ -2268,7 +2269,7 @@ filt_aioattach(struct knote *kn) static void filt_aiodetach(struct knote *kn) { - struct aiocblist *aiocbe = (struct aiocblist *)kn->kn_sdata; + struct aiocblist *aiocbe = kn->kn_ptr.p_aio; knlist_remove(&aiocbe->klist, kn, 0); } @@ -2278,7 +2279,7 @@ filt_aiodetach(struct knote *kn) static int filt_aio(struct knote *kn, long hint) { - struct aiocblist *aiocbe = (struct aiocblist *)kn->kn_sdata; + struct aiocblist *aiocbe = kn->kn_ptr.p_aio; kn->kn_data = aiocbe->uaiocb._aiocb_private.error; if (aiocbe->jobstate != JOBST_JOBFINISHED && --- sys/sys/event.h.orig 2005-07-02 01:28:32.000000000 +0900 +++ sys/sys/event.h 2007-12-21 06:04:18.000000000 +0900 @@ -186,6 +186,7 @@ struct knote { union { struct file *p_fp; /* file data pointer */ struct proc *p_proc; /* proc pointer */ + struct aiocblist *p_aio; /* AIO job pointer */ } kn_ptr; struct filterops *kn_fop; void *kn_hook; In case above patches are not good, here is an ad-hoc workaround: --- src/kern/kern_event.c.orig 2007-07-15 06:23:30.000000000 +0900 +++ src/kern/kern_event.c 2007-12-21 00:27:55.000000000 +0900 @@ -924,6 +924,17 @@ KN_LIST_LOCK(kn); } else { /* + * XXX FVFILT_AIO does not suppose that knote + * will ever be modified with user-supplied value. + */ + if ((filt == EVFILT_AIO || filt == EVFILT_LIO) && + (kev->flags & EV_FLAG1) == 0) { + KQ_UNLOCK(kq); + error = EPERM; + goto done; + } + + /* * The user may change some filter values after the * initial EV_ADD, but doing so will not reset any * filter which has already been triggered. >Release-Note: >Audit-Trail: >Unformatted: