-
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