Print this page
MFV: illumos-gate@68c34d0407d130a7e8cb7dfb5394a985db03d785
9951 hook_stack_notify_unregister can leave stack locked
Reviewed by: Andy Fiddaman <omnios@citrus-it.net>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Yuri Pankov <yuripv@yuripv.net>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Jason King <jason.king@joyent.com>

Split Close
Expand all
Collapse all
          --- old/usr/src/uts/common/io/hook.c
          +++ new/usr/src/uts/common/io/hook.c
↓ open down ↓ 14 lines elided ↑ open up ↑
  15   15   * If applicable, add the following below this CDDL HEADER, with the
  16   16   * fields enclosed by brackets "[]" replaced with your own identifying
  17   17   * information: Portions Copyright [yyyy] [name of copyright owner]
  18   18   *
  19   19   * CDDL HEADER END
  20   20   */
  21   21  /*
  22   22   * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  23   23   * Use is subject to license terms.
  24   24   *
  25      - * Copyright 2013 Joyent, Inc.  All rights reserved.
       25 + * Copyright 2018 Joyent, Inc.  All rights reserved.
  26   26   * Copyright (c) 2016 by Delphix. All rights reserved.
  27   27   */
  28   28  #include <sys/param.h>
  29   29  #include <sys/types.h>
  30   30  #include <sys/systm.h>
  31   31  #include <sys/errno.h>
  32   32  #include <sys/kmem.h>
  33   33  #include <sys/mutex.h>
  34   34  #include <sys/condvar.h>
  35   35  #include <sys/modctl.h>
↓ open down ↓ 664 lines elided ↑ open up ↑
 700  700   * callbacks to activiate when protocols are added/deleted.
 701  701   * As with hook_stack_notify_register, if all things are going well then
 702  702   * a fake unregister event is delivered to the callback being removed
 703  703   * for each hook family that presently exists.
 704  704   */
 705  705  int
 706  706  hook_stack_notify_unregister(netstackid_t stackid, hook_notify_fn_t callback)
 707  707  {
 708  708          hook_family_int_t *hfi;
 709  709          hook_stack_t *hks;
 710      -        boolean_t canrun;
 711  710          char buffer[16];
 712  711          void *arg;
 713  712          int error;
 714  713  
 715  714          mutex_enter(&hook_stack_lock);
 716  715          hks = hook_stack_get(stackid);
 717      -        if (hks != NULL) {
 718      -                CVW_ENTER_WRITE(&hks->hks_lock);
 719      -                canrun = (hook_wait_setflag(&hks->hks_waiter, FWF_ADD_WAIT_MASK,
 720      -                    FWF_ADD_WANTED, FWF_ADD_ACTIVE) != -1);
      716 +        if (hks == NULL) {
      717 +                mutex_exit(&hook_stack_lock);
      718 +                return (ESRCH);
      719 +        }
 721  720  
 722      -                error = hook_notify_unregister(&hks->hks_nhead, callback, &arg);
      721 +        CVW_ENTER_WRITE(&hks->hks_lock);
      722 +        /*
      723 +         * If hook_wait_setflag returns -1, another thread has flagged that it
      724 +         * is attempting to destroy this hook stack.  Before it can flag that
      725 +         * it's destroying the hook stack, it must first verify (with
      726 +         * hook_stack_lock held) that the hook stack is empty.  If we
      727 +         * encounter this, it means we should have nothing to do and we
      728 +         * just snuck in.
      729 +         */
      730 +        if (hook_wait_setflag(&hks->hks_waiter, FWF_DEL_WAIT_MASK,
      731 +            FWF_DEL_WANTED, FWF_DEL_ACTIVE) == -1) {
      732 +                VERIFY(TAILQ_EMPTY(&hks->hks_nhead));
 723  733                  CVW_EXIT_WRITE(&hks->hks_lock);
 724      -        } else {
 725      -                error = ESRCH;
      734 +                mutex_exit(&hook_stack_lock);
      735 +                return (ESRCH);
 726  736          }
      737 +
      738 +        error = hook_notify_unregister(&hks->hks_nhead, callback, &arg);
      739 +        CVW_EXIT_WRITE(&hks->hks_lock);
 727  740          mutex_exit(&hook_stack_lock);
 728  741  
 729  742          if (error == 0) {
 730      -                if (canrun) {
 731      -                        /*
 732      -                         * Generate fake unregister event for callback that
 733      -                         * is being removed, letting it know everything that
 734      -                         * currently exists is now "disappearing."
 735      -                         */
 736      -                        (void) snprintf(buffer, sizeof (buffer), "%u",
 737      -                            hks->hks_netstackid);
      743 +                /*
      744 +                 * Generate fake unregister event for callback that
      745 +                 * is being removed, letting it know everything that
      746 +                 * currently exists is now "disappearing."
      747 +                 */
      748 +                (void) snprintf(buffer, sizeof (buffer), "%u",
      749 +                    hks->hks_netstackid);
 738  750  
 739      -                        SLIST_FOREACH(hfi, &hks->hks_familylist, hfi_entry) {
 740      -                                callback(HN_UNREGISTER, arg, buffer, NULL,
 741      -                                    hfi->hfi_family.hf_name);
 742      -                        }
 743      -
 744      -                        hook_wait_unsetflag(&hks->hks_waiter, FWF_ADD_ACTIVE);
      751 +                SLIST_FOREACH(hfi, &hks->hks_familylist, hfi_entry) {
      752 +                        callback(HN_UNREGISTER, arg, buffer, NULL,
      753 +                            hfi->hfi_family.hf_name);
 745  754                  }
 746      -
 747      -                mutex_enter(&hook_stack_lock);
 748      -                hks = hook_stack_get(stackid);
 749      -                if ((error == 0) && (hks->hks_shutdown == 2))
 750      -                        hook_stack_remove(hks);
 751      -                mutex_exit(&hook_stack_lock);
      755 +        } else {
      756 +                /*
      757 +                 * hook_notify_unregister() should only fail if the callback has
      758 +                 * already been deleted (ESRCH).
      759 +                 */
      760 +                VERIFY3S(error, ==, ESRCH);
 752  761          }
 753  762  
      763 +        mutex_enter(&hook_stack_lock);
      764 +        hook_wait_unsetflag(&hks->hks_waiter, FWF_DEL_ACTIVE);
      765 +        if (hks->hks_shutdown == 2)
      766 +                hook_stack_remove(hks);
      767 +        mutex_exit(&hook_stack_lock);
      768 +
 754  769          return (error);
 755  770  }
 756  771  
 757  772  /*
 758  773   * Function:    hook_stack_notify_run
 759  774   * Returns:     None
 760  775   * Parameters:  hks(I)  - hook stack pointer to execute callbacks for
 761  776   *              name(I) - name of a hook family
 762  777   *              cmd(I)  - either HN_UNREGISTER or HN_REGISTER
 763  778   *
↓ open down ↓ 355 lines elided ↑ open up ↑
1119 1134  
1120 1135          return (new);
1121 1136  }
1122 1137  
1123 1138  /*
1124 1139   * Function:    hook_family_find
1125 1140   * Returns:     internal family pointer - NULL = Not match
1126 1141   * Parameters:  family(I) - family name string
1127 1142   *
1128 1143   * Search family list with family name
1129      - *      A lock on hfi_lock must be held when called.
     1144 + *      A lock on hfi_lock must be held when called.
1130 1145   */
1131 1146  static hook_family_int_t *
1132 1147  hook_family_find(char *family, hook_stack_t *hks)
1133 1148  {
1134 1149          hook_family_int_t *hfi = NULL;
1135 1150  
1136 1151          ASSERT(family != NULL);
1137 1152  
1138 1153          SLIST_FOREACH(hfi, &hks->hks_familylist, hfi_entry) {
1139 1154                  if (strcmp(hfi->hfi_family.hf_name, family) == 0)
↓ open down ↓ 504 lines elided ↑ open up ↑
1644 1659          return (new);
1645 1660  }
1646 1661  
1647 1662  /*
1648 1663   * Function:    hook_event_find
1649 1664   * Returns:     internal event pointer - NULL = Not match
1650 1665   * Parameters:  hfi(I)   - internal family pointer
1651 1666   *              event(I) - event name string
1652 1667   *
1653 1668   * Search event list with event name
1654      - *      A lock on hfi->hfi_lock must be held when called.
     1669 + *      A lock on hfi->hfi_lock must be held when called.
1655 1670   */
1656 1671  static hook_event_int_t *
1657 1672  hook_event_find(hook_family_int_t *hfi, char *event)
1658 1673  {
1659 1674          hook_event_int_t *hei = NULL;
1660 1675  
1661 1676          ASSERT(hfi != NULL);
1662 1677          ASSERT(event != NULL);
1663 1678  
1664 1679          SLIST_FOREACH(hei, &hfi->hfi_head, hei_entry) {
↓ open down ↓ 902 lines elided ↑ open up ↑
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX