Skip to content
  • Rick Macklem's avatar
    nfscl: Fix a use after free in nfscl_cleanupkext() · dd08b84e
    Rick Macklem authored
    ler@, markj@ reported a use after free in nfscl_cleanupkext().
    They also provided two possible causes:
    - In nfscl_cleanup_common(), "own" is the owner string
      owp->nfsow_owner.  If we free that particular
      owner structure, than in subsequent comparisons
      "own" will point to freed memory.
    - nfscl_cleanup_common() can free more than one owner, so the use
      of LIST_FOREACH_SAFE() in nfscl_cleanupkext() is not sufficient.
    
    I also believe there is a 3rd:
    - If nfscl_freeopenowner() or nfscl_freelockowner() is called
      without the NFSCLSTATE mutex held, this could race with
      nfscl_cleanupkext().
      This could happen when the exclusive lock is held
      on the client, such as when delegations are being returned.
    
    This patch fixes them as follows:
    1 - Copy the owner string to a local variable before the
        nfscl_cleanup_common() call.
    2 - Modify nfscl_cleanup_common() to return whether or not a
        free was done.
        When a free was done, do a goto to restart the loop, instead
        of using FOREACH_SAFE, which was not safe in this case.
    3 - Acquire the NFSCLSTATE mutex in nfscl_freeopenowner()
        and nfscl_freelockowner(), if it not already held.
        This serializes all of these calls with the ones done in
        nfscl_cleanup_common().
    
    Reported by:	ler
    Reviewed by:	markj
    MFC after:	1 week
    Differential Revision:	https://reviews.freebsd.org/D34334
    dd08b84e