Fix some FreeBSD VOPs to synchronize properly with teardown #12704
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The objset object is reallocated during certain dataset operations, such as rollbacks, so the objset pointer must be loaded after acquiring the teardown lock. Signed-off-by: Mark Johnston <markj@FreeBSD.org>
We have to hold the teardown lock while dereferencing zfsvfs->z_os and, I believe, when committing to the ZIL. Note that jumping to the "out" label, "error" is always non-zero. Signed-off-by: Mark Johnston <markj@FreeBSD.org>
|
Thanks. The stable/12 build failure is unrelated and should be fixed by a buildbot config update I have opened a PR for, so don't worry about that :) |
ghost
approved these changes
Oct 28, 2021
behlendorf
approved these changes
Oct 29, 2021
behlendorf
pushed a commit
that referenced
this pull request
Oct 29, 2021
We have to hold the teardown lock while dereferencing zfsvfs->z_os and, I believe, when committing to the ZIL. Note that jumping to the "out" label, "error" is always non-zero. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ryan Moeller <ryan@ixsystems.com> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes #12704
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 13, 2021
The objset object is reallocated during certain dataset operations, such as rollbacks, so the objset pointer must be loaded after acquiring the teardown lock. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ryan Moeller <ryan@ixsystems.com> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12704
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 13, 2021
We have to hold the teardown lock while dereferencing zfsvfs->z_os and, I believe, when committing to the ZIL. Note that jumping to the "out" label, "error" is always non-zero. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ryan Moeller <ryan@ixsystems.com> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12704
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 13, 2021
The objset object is reallocated during certain dataset operations, such as rollbacks, so the objset pointer must be loaded after acquiring the teardown lock. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ryan Moeller <ryan@ixsystems.com> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12704
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 13, 2021
We have to hold the teardown lock while dereferencing zfsvfs->z_os and, I believe, when committing to the ZIL. Note that jumping to the "out" label, "error" is always non-zero. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ryan Moeller <ryan@ixsystems.com> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12704
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
This change fixes a kernel use-after-free encountered while testing a fix for a deadlock in zfs_getpages(). After fixing the problem in zfs_getpages(), I noticed the same bug in a couple of other VOPs, specifically setattr and rename.
Description
The pointer to the objset for a dataset must be loaded while in a ZFS_ENTER/EXIT block. Otherwise a concurrent rollback or recv can invalidate the pointer, resulting in a use after free when VOPs resume (assuming that they don't fail immediately).
How Has This Been Tested?
I have a stress test for the aforementioned deadlock. It simply consists of running kernel builds in a snapshotted dataset, and rolling back the dataset periodically. It exposed the use-after-free in zfs_getpages() very quickly, but now runs without issues. I've also run the change on a desktop system which uses ZFS-on-root, and on a host running syzkaller (which on FreeBSD uses ZFS to create VM image snapshots and clones, so is a decent regression test).
Types of changes
Checklist:
Signed-off-by.