Bug #9951

hook_stack_notify_unregister can leave stack locked

Added by Jason King 4 months ago. Updated 4 months ago.

Status:ClosedStart date:2018-11-02
Priority:NormalDue date:
Assignee:Jason King% Done:

100%

Category:kernel
Target version:-
Difficulty:Medium Tags:needs-triage

Description

Upstream of Joyent OS-7318.

In hook_stack_notify_unregister, we have the following code:

        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);

                error = hook_notify_unregister(&hks->hks_nhead, callback, &arg);
                CVW_EXIT_WRITE(&hks->hks_lock);
        } else {
                error = ESRCH;
        }
        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);
                        }

                        hook_wait_unsetflag(&hks->hks_waiter, FWF_ADD_ACTIVE);
                }

If the hook_notify_unregister call fails, hook_wait_unsetflag will never get cleared, leaving the hook stack locked.
Also, as this is deleting data, the flags should be FWF_DEL_{WAIT_MASK,WANTED,ACTIVE}

History

#1 Updated by Jason King 4 months ago

  • Status changed from New to Pending RTI

#2 Updated by Electric Monk 4 months ago

  • % Done changed from 0 to 100
  • Status changed from Pending RTI to Closed

git commit 68c34d0407d130a7e8cb7dfb5394a985db03d785

commit  68c34d0407d130a7e8cb7dfb5394a985db03d785
Author: Jason King <jason.king@joyent.com>
Date:   2018-11-06T19:03:56.000Z

    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>

Also available in: Atom