HomeFreeBSD

nfscl: Fix a use after free in nfscl_cleanupkext()

Description

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 or when recovering from NFSERR_EXPIRED.

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() so that it will never free more

than the first matching element.  Normally there should only
be one element in each list with a matching open/lock owner
anyhow (but there might be a bug that results in a duplicate).
This should guarantee that the FOREACH_SAFE loops in
nfscl_cleanupkext() are adequate.

3 - Acquire the NFSCLSTATE mutex in nfscl_freeopenowner()

and nfscl_freelockowner(), if it is not already held.
This serializes all of these calls with the ones done in
nfscl_cleanup_common().

(cherry picked from commit 1cedb4ea1a790f976ec6211c938dfaa23874b497)

Details

Provenance
rmacklemAuthored on Feb 25 2022, 3:27 PM
Parents
rGaec30b5b2dcf: ssh: correct configure option name
Branches
Unknown
Tags
Unknown