From 68c34d0407d130a7e8cb7dfb5394a985db03d785 Mon Sep 17 00:00:00 2001 From: Jason King Date: Mon, 22 Oct 2018 00:53:14 +0000 Subject: 9951 hook_stack_notify_unregister can leave stack locked Reviewed by: Andy Fiddaman Reviewed by: Igor Kozhukhov Reviewed by: Yuri Pankov Approved by: Dan McDonald --- usr/src/uts/common/io/hook.c | 77 ++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/usr/src/uts/common/io/hook.c b/usr/src/uts/common/io/hook.c index 2edbe752ce..c3ebfa0e47 100644 --- a/usr/src/uts/common/io/hook.c +++ b/usr/src/uts/common/io/hook.c @@ -22,7 +22,7 @@ * Copyright 2008 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. * - * Copyright 2013 Joyent, Inc. All rights reserved. + * Copyright 2018 Joyent, Inc. All rights reserved. * Copyright (c) 2016 by Delphix. All rights reserved. */ #include @@ -707,50 +707,65 @@ hook_stack_notify_unregister(netstackid_t stackid, hook_notify_fn_t callback) { hook_family_int_t *hfi; hook_stack_t *hks; - boolean_t canrun; char buffer[16]; void *arg; int error; mutex_enter(&hook_stack_lock); hks = hook_stack_get(stackid); - if (hks != NULL) { - CVW_ENTER_WRITE(&hks->hks_lock); - canrun = (hook_wait_setflag(&hks->hks_waiter, FWF_ADD_WAIT_MASK, - FWF_ADD_WANTED, FWF_ADD_ACTIVE) != -1); + if (hks == NULL) { + mutex_exit(&hook_stack_lock); + return (ESRCH); + } - error = hook_notify_unregister(&hks->hks_nhead, callback, &arg); + CVW_ENTER_WRITE(&hks->hks_lock); + /* + * If hook_wait_setflag returns -1, another thread has flagged that it + * is attempting to destroy this hook stack. Before it can flag that + * it's destroying the hook stack, it must first verify (with + * hook_stack_lock held) that the hook stack is empty. If we + * encounter this, it means we should have nothing to do and we + * just snuck in. + */ + if (hook_wait_setflag(&hks->hks_waiter, FWF_DEL_WAIT_MASK, + FWF_DEL_WANTED, FWF_DEL_ACTIVE) == -1) { + VERIFY(TAILQ_EMPTY(&hks->hks_nhead)); CVW_EXIT_WRITE(&hks->hks_lock); - } else { - error = ESRCH; + mutex_exit(&hook_stack_lock); + return (ESRCH); } + + error = hook_notify_unregister(&hks->hks_nhead, callback, &arg); + CVW_EXIT_WRITE(&hks->hks_lock); mutex_exit(&hook_stack_lock); if (error == 0) { - if (canrun) { - /* - * Generate fake unregister event for callback that - * is being removed, letting it know everything that - * currently exists is now "disappearing." - */ - (void) snprintf(buffer, sizeof (buffer), "%u", - hks->hks_netstackid); - - SLIST_FOREACH(hfi, &hks->hks_familylist, hfi_entry) { - callback(HN_UNREGISTER, arg, buffer, NULL, - hfi->hfi_family.hf_name); - } + /* + * Generate fake unregister event for callback that + * is being removed, letting it know everything that + * currently exists is now "disappearing." + */ + (void) snprintf(buffer, sizeof (buffer), "%u", + hks->hks_netstackid); - hook_wait_unsetflag(&hks->hks_waiter, FWF_ADD_ACTIVE); + SLIST_FOREACH(hfi, &hks->hks_familylist, hfi_entry) { + callback(HN_UNREGISTER, arg, buffer, NULL, + hfi->hfi_family.hf_name); } - - mutex_enter(&hook_stack_lock); - hks = hook_stack_get(stackid); - if ((error == 0) && (hks->hks_shutdown == 2)) - hook_stack_remove(hks); - mutex_exit(&hook_stack_lock); + } else { + /* + * hook_notify_unregister() should only fail if the callback has + * already been deleted (ESRCH). + */ + VERIFY3S(error, ==, ESRCH); } + mutex_enter(&hook_stack_lock); + hook_wait_unsetflag(&hks->hks_waiter, FWF_DEL_ACTIVE); + if (hks->hks_shutdown == 2) + hook_stack_remove(hks); + mutex_exit(&hook_stack_lock); + return (error); } @@ -1126,7 +1141,7 @@ hook_family_copy(hook_family_t *src) * Parameters: family(I) - family name string * * Search family list with family name - * A lock on hfi_lock must be held when called. + * A lock on hfi_lock must be held when called. */ static hook_family_int_t * hook_family_find(char *family, hook_stack_t *hks) @@ -1651,7 +1666,7 @@ hook_event_copy(hook_event_t *src) * event(I) - event name string * * Search event list with event name - * A lock on hfi->hfi_lock must be held when called. + * A lock on hfi->hfi_lock must be held when called. */ static hook_event_int_t * hook_event_find(hook_family_int_t *hfi, char *event) -- cgit v1.2.3 From cf25223258f3cd568605b3e10c1432e5e93b2c5e Mon Sep 17 00:00:00 2001 From: Andy Fiddaman Date: Wed, 31 Oct 2018 15:19:00 +0000 Subject: 9941 Noise from cfgadm plugins Reviewed by: Toomas Soome Reviewed by: Gergő Mihály Doma Approved by: Dan McDonald MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- usr/src/lib/cfgadm_plugins/ib/common/cfga_ib.c | 9 ++++----- usr/src/lib/cfgadm_plugins/sata/common/cfga_sata.c | 7 +++---- usr/src/lib/cfgadm_plugins/usb/common/cfga_usb.c | 9 ++++----- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/usr/src/lib/cfgadm_plugins/ib/common/cfga_ib.c b/usr/src/lib/cfgadm_plugins/ib/common/cfga_ib.c index 6186f187e0..e9585ef40a 100644 --- a/usr/src/lib/cfgadm_plugins/ib/common/cfga_ib.c +++ b/usr/src/lib/cfgadm_plugins/ib/common/cfga_ib.c @@ -21,6 +21,7 @@ /* * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. + * Copyright 2018 OmniOS Community Edition (OmniOSce) Association. */ #include "cfga_ib.h" @@ -73,7 +74,7 @@ void cfga_msg(struct cfga_msg *, const char *); cfga_err_t cfga_help(struct cfga_msg *, const char *, cfga_flags_t); static int ib_confirm(struct cfga_confirm *, char *); -static char *ib_get_devicepath(const char *); +static char *ib_get_devicepath(const char *); /* External function prototypes */ @@ -194,7 +195,7 @@ static char *ib_service_subopts[] = { /* Communication Service name : "port" or "vppa" or "hca-svc" */ static char *comm_name = NULL; -char *service_name = NULL; /* service name */ +char *service_name = NULL; /* service name */ ib_service_type_t service_type = IB_NONE; /* service type */ @@ -1654,14 +1655,12 @@ cfga_list_ext(const char *ap_id, cfga_list_data_t **ap_id_list, int *nlistp, cfga_list_data_t *clp = NULL; if ((rv = ib_verify_params(ap_id, options, errstring)) != CFGA_IB_OK) { - (void) cfga_help(NULL, options, flags); return (ib_err_msg(errstring, rv, ap_id, errno)); } /* make sure we have a valid ap_id_list */ if (ap_id_list == NULL || nlistp == NULL) { DPRINTF("cfga_list_ext: list = NULL or nlistp = NULL\n"); - (void) cfga_help(NULL, options, flags); return (ib_err_msg(errstring, CFGA_IB_INVAL_ARG_ERR, ap_id, errno)); } @@ -1717,7 +1716,7 @@ cfga_list_ext(const char *ap_id, cfga_list_data_t **ap_id_list, int *nlistp, /* * *nlistp contains to how many APIDs to show w/ cfgadm -l. * If ap_id is "fabric" then - * *nlistp is all Dynamic Apids + One more for "fabric" + * *nlistp is all Dynamic Apids + One more for "fabric" * If ap_id is "HCA" ap_id then * *nlistp is 1 * Note that each HCA is a static APID, so nlistp will be 1 always diff --git a/usr/src/lib/cfgadm_plugins/sata/common/cfga_sata.c b/usr/src/lib/cfgadm_plugins/sata/common/cfga_sata.c index 29f059b2db..ad4dcd6dd3 100644 --- a/usr/src/lib/cfgadm_plugins/sata/common/cfga_sata.c +++ b/usr/src/lib/cfgadm_plugins/sata/common/cfga_sata.c @@ -22,6 +22,7 @@ /* * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. + * Copyright 2018 OmniOS Community Edition (OmniOSce) Association. */ #include @@ -570,7 +571,7 @@ cfga_change_state( cfga_flags_t flags) { int ret; - int len; + int len; char *msg; char *devpath; nvlist_t *nvl = NULL; @@ -922,7 +923,7 @@ cfga_private_func( cfga_flags_t flags) { int len; - char *msg; + char *msg; nvlist_t *list = NULL; ap_ostate_t ostate; ap_rstate_t rstate; @@ -1430,7 +1431,6 @@ cfga_list_ext( if ((rv = verify_params(ap_id, options, errstring)) != CFGA_SATA_OK) { - (void) cfga_help(NULL, options, flags); goto bailout; } /* We do not care here about dynamic AP name component */ @@ -1440,7 +1440,6 @@ cfga_list_ext( if (ap_id_list == NULL || nlistp == NULL) { rv = CFGA_SATA_DATA_ERROR; - (void) cfga_help(NULL, options, flags); goto bailout; } diff --git a/usr/src/lib/cfgadm_plugins/usb/common/cfga_usb.c b/usr/src/lib/cfgadm_plugins/usb/common/cfga_usb.c index e570daeeb3..cfb0b63232 100644 --- a/usr/src/lib/cfgadm_plugins/usb/common/cfga_usb.c +++ b/usr/src/lib/cfgadm_plugins/usb/common/cfga_usb.c @@ -21,6 +21,7 @@ /* * Copyright 2008 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. + * Copyright 2018 OmniOS Community Edition (OmniOSce) Association. */ @@ -36,7 +37,7 @@ extern cfga_usb_ret_t usb_rcm_online(const char *, char **, char *, extern cfga_usb_ret_t usb_rcm_remove(const char *, char **, char *, cfga_flags_t); static int usb_confirm(struct cfga_confirm *, char *); -static char *usb_get_devicepath(const char *); +static char *usb_get_devicepath(const char *); /* * This file contains the entry points to the plugin as defined in the @@ -621,7 +622,7 @@ device_connected(devctl_hdl_t hdl, nvlist_t *list, ap_ostate_t *ostate) */ cfga_usb_ret_t do_control_ioctl(const char *ap_id, uint_t subcommand, uint_t arg, - void **descrp, size_t *sizep) + void **descrp, size_t *sizep) { int fd = -1; uint_t port; @@ -1224,7 +1225,7 @@ cfga_private_func( char *msg; nvlist_t *list = NULL; ap_ostate_t ostate; - devctl_hdl_t hdl = NULL; + devctl_hdl_t hdl = NULL; cfga_usb_ret_t rv; usb_dev_descr_t *dev_descrp = NULL; char *driver = NULL; @@ -1474,14 +1475,12 @@ cfga_list_ext( DPRINTF("cfga_list_ext:\n"); if ((rv = verify_params(ap_id, options, errstring)) != CFGA_USB_OK) { - (void) cfga_help(NULL, options, flags); goto bailout; } if (ap_id_list == NULL || nlistp == NULL) { DPRINTF("cfga_list_ext: list = NULL or nlistp = NULL\n"); rv = CFGA_USB_INTERNAL_ERROR; - (void) cfga_help(NULL, options, flags); goto bailout; } -- cgit v1.2.3