]> CyberLeo.Net >> Repos - FreeBSD/FreeBSD.git/commit
nfscl: Fix a use after free in nfscl_cleanupkext()
authorRick Macklem <rmacklem@FreeBSD.org>
Tue, 22 Feb 2022 22:21:43 +0000 (14:21 -0800)
committerRick Macklem <rmacklem@FreeBSD.org>
Tue, 22 Feb 2022 22:21:43 +0000 (14:21 -0800)
commitdd08b84e35b6fdee0df5fd0e0533cd361dec71cb
tree128835d815e548218fa470ce82810faf770f0017
parent33ad990ce974be50abdc25427b0f365699d83a34
nfscl: Fix a use after free in nfscl_cleanupkext()

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
sys/fs/nfsclient/nfs_clstate.c