# HG changeset patch # User Marcel Telka # Date 1360354072 18000 # Node ID a604ff203021b2408d1696dbe1eb36a9e10613e1 # Parent 4f6a155f70feb9f6f6ac616a7da108f9013857be 3531 Race between log_sysevent_filename() and log_event_upcall() can cause panic Reviewed by: Gordon Ross Reviewed by: Dan McDonald Reviewed by: Albert Lee Approved by: Richard Lowe diff -r 4f6a155f70fe -r a604ff203021 usr/src/uts/common/os/log_sysevent.c --- a/usr/src/uts/common/os/log_sysevent.c Fri Feb 08 13:19:22 2013 -0800 +++ b/usr/src/uts/common/os/log_sysevent.c Fri Feb 08 15:07:52 2013 -0500 @@ -20,6 +20,7 @@ */ /* * Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright 2013 Nexenta Systems, Inc. All rights reserved. */ #include @@ -95,10 +96,10 @@ static int log_event_delivery = LOGEVENT_DELIVERY_HOLD; -static char *logevent_door_upcall_filename = NULL; -static int logevent_door_upcall_filename_size; +static char logevent_door_upcall_filename[MAXPATHLEN]; static door_handle_t event_door = NULL; /* Door for upcalls */ +static kmutex_t event_door_mutex; /* To protect event_door */ /* * async thread-related variables @@ -146,33 +147,6 @@ static kcondvar_t event_pause_cv; static int event_pause_state = 0; -/* - * log_event_upcall_lookup - Establish door connection with user event - * daemon (syseventd) - */ -static int -log_event_upcall_lookup() -{ - int error; - - if (event_door) { /* Release our previous hold (if any) */ - door_ki_rele(event_door); - } - - event_door = NULL; - - /* - * Locate the door used for upcalls - */ - if ((error = - door_ki_open(logevent_door_upcall_filename, &event_door)) != 0) { - return (error); - } - - return (0); -} - - /*ARGSUSED*/ static void log_event_busy_timeout(void *arg) @@ -234,23 +208,44 @@ darg.desc_ptr = NULL; darg.desc_num = 0; - if ((event_door == NULL) && - ((error = log_event_upcall_lookup()) != 0)) { - LOG_DEBUG((CE_CONT, - "log_event_upcall: event_door error (%d)\n", error)); - - return (error); - } - LOG_DEBUG1((CE_CONT, "log_event_upcall: 0x%llx\n", (longlong_t)SE_SEQ((sysevent_t *)&arg->buf))); save_arg = darg; for (retry = 0; ; retry++) { + + mutex_enter(&event_door_mutex); + if (event_door == NULL) { + mutex_exit(&event_door_mutex); + + return (EBADF); + } + if ((error = door_ki_upcall_limited(event_door, &darg, NULL, SIZE_MAX, 0)) == 0) { + mutex_exit(&event_door_mutex); break; } + + /* + * EBADF is handled outside the switch below because we need to + * hold event_door_mutex a bit longer + */ + if (error == EBADF) { + /* Server died */ + door_ki_rele(event_door); + event_door = NULL; + + mutex_exit(&event_door_mutex); + return (error); + } + + mutex_exit(&event_door_mutex); + + /* + * The EBADF case is already handled above with event_door_mutex + * held + */ switch (error) { case EINTR: neintr++; @@ -266,24 +261,6 @@ nticks = LOG_EVENT_MAX_PAUSE; darg = save_arg; break; - case EBADF: - LOG_DEBUG((CE_CONT, "log_event_upcall: rebinding\n")); - /* Server may have died. Try rebinding */ - if ((error = log_event_upcall_lookup()) != 0) { - LOG_DEBUG((CE_CONT, - "log_event_upcall: lookup error %d\n", - error)); - return (EBADF); - } - if (retry > 4) { - LOG_DEBUG((CE_CONT, - "log_event_upcall: ebadf\n")); - return (EBADF); - } - LOG_DEBUG((CE_CONT, "log_event_upcall: " - "retrying upcall after lookup\n")); - darg = save_arg; - break; default: cmn_err(CE_CONT, "log_event_upcall: door_ki_upcall error %d\n", @@ -347,17 +324,17 @@ q = log_eventq_head; while (q) { - log_eventq_t *next; + if (log_event_delivery == LOGEVENT_DELIVERY_HOLD) { + upcall_err = EAGAIN; + break; + } + + log_event_delivery = LOGEVENT_DELIVERY_OK; /* * Release event queue lock during upcall to * syseventd */ - if (log_event_delivery == LOGEVENT_DELIVERY_HOLD) { - upcall_err = EAGAIN; - break; - } - mutex_exit(&eventq_head_mutex); if ((upcall_err = log_event_upcall(&q->arg)) != 0) { mutex_enter(&eventq_head_mutex); @@ -390,6 +367,8 @@ LOG_DEBUG((CE_CONT, "log_event_deliver: " "door upcall/daemon restart race\n")); } else { + log_eventq_t *next; + /* * Move the event to the sent queue when a * successful delivery has been made. @@ -426,12 +405,9 @@ * resumption, continue. Otherwise, we wait until * we are signaled to continue. */ - if (log_event_delivery == LOGEVENT_DELIVERY_CONT) { - log_event_delivery = LOGEVENT_DELIVERY_OK; + if (log_event_delivery == LOGEVENT_DELIVERY_CONT) continue; - } else { - log_event_delivery = LOGEVENT_DELIVERY_HOLD; - } + log_event_delivery = LOGEVENT_DELIVERY_HOLD; LOG_DEBUG1((CE_CONT, "log_event_deliver: EAGAIN\n")); break; @@ -465,6 +441,8 @@ void log_event_init() { + mutex_init(&event_door_mutex, NULL, MUTEX_DEFAULT, NULL); + mutex_init(&eventq_head_mutex, NULL, MUTEX_DEFAULT, NULL); mutex_init(&eventq_sent_mutex, NULL, MUTEX_DEFAULT, NULL); cv_init(&log_event_cv, NULL, CV_DEFAULT, NULL); @@ -1601,23 +1579,19 @@ int log_sysevent_filename(char *file) { - /* - * Called serially by syseventd init code, no need to protect door - * data. - */ + mutex_enter(&event_door_mutex); + + (void) strlcpy(logevent_door_upcall_filename, file, + sizeof (logevent_door_upcall_filename)); + /* Unbind old event door */ - if (logevent_door_upcall_filename) { - kmem_free(logevent_door_upcall_filename, - logevent_door_upcall_filename_size); - if (event_door) { - door_ki_rele(event_door); - event_door = NULL; - } - } - logevent_door_upcall_filename_size = strlen(file) + 1; - logevent_door_upcall_filename = kmem_alloc( - logevent_door_upcall_filename_size, KM_SLEEP); - (void) strcpy(logevent_door_upcall_filename, file); + if (event_door != NULL) + door_ki_rele(event_door); + /* Establish door connection with user event daemon (syseventd) */ + if (door_ki_open(logevent_door_upcall_filename, &event_door) != 0) + event_door = NULL; + + mutex_exit(&event_door_mutex); /* * We are called when syseventd restarts. Move all sent, but