Date: Thu, 20 Dec 2007 23:16:58 GMT From: MOROHOSHI Akihiko <moro@remus.dti.ne.jp> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/118911: kevent can cause the system to crash if aio is enabled. Message-ID: <200712202316.lBKNGwXl016470@www.freebsd.org> Resent-Message-ID: <200712202320.lBKNK1Ea082562@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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 <aio.h>
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/event.h>
#include <sys/time.h>
#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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200712202316.lBKNGwXl016470>