ZAP: Fix leaf references on zap_expand_leaf() errors #16159
Merged
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
Depending on kind of error zap_expand_leaf() may return with or without valid leaf reference held. Make sure it returns NULL if due to error it has no leaf to return. Make its callers to check the returned leaf pointer, and release the leaf if it is not NULL. Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes openzfs#12366
snajpa
reviewed
May 3, 2024
| zap_put_leaf_maybe_grow_ptrtbl(zn, l, tag, tx); | ||
| if (l != NULL) { | ||
| if (err == ENOSPC) | ||
| zap_put_leaf(l); |
There was a problem hiding this comment.
(not a review, I'm trying to understand this)
it gets here because it was zap_grow_ptrtbl which returned ENOSPC; when can that happen? a) the ZAP is way too large (2^46 entries, right?) and b) on transition from 2^28 to 2^48 zap_hashbits, that is when upgrading from micro to fat zap? Is there any other case that I'm missing why ENOSPC can happen here?
I mean, I'm trying to understand which separate two leaves does the zap_put_leaf get called for and whether that could cause additional trouble - this is the second occasion, the first is on line 713.
There was a problem hiding this comment.
I've primarily just refactored already present code. I was actually thinking about actual out of space, but it seems not checked at this point, and you seem right that hash size limit may be the primary (and pretty unlikely) factor.
zap_expand_leaf() puts one leaf inside, while another (where the entry is going to be inserted) returns to the caller. So it is the leaf that gets new entry is checked for coming overflow and possible hash table grow. I.e. I see no problems, unless you specify better.
behlendorf
approved these changes
May 10, 2024
behlendorf
added
Status: Accepted
amotin
added a commit
to amotin/zfs
that referenced
this pull request
May 23, 2024
Depending on kind of error zap_expand_leaf() may return with or without valid leaf reference held. Make sure it returns NULL if due to error it has no leaf to return. Make its callers to check the returned leaf pointer, and release the leaf if it is not NULL. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes openzfs#12366 Closes openzfs#16159
ixhamza
pushed a commit
to truenas/zfs
that referenced
this pull request
May 23, 2024
Depending on kind of error zap_expand_leaf() may return with or without valid leaf reference held. Make sure it returns NULL if due to error it has no leaf to return. Make its callers to check the returned leaf pointer, and release the leaf if it is not NULL. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes openzfs#12366 Closes openzfs#16159
ixhamza
pushed a commit
to truenas/zfs
that referenced
this pull request
May 23, 2024
Depending on kind of error zap_expand_leaf() may return with or without valid leaf reference held. Make sure it returns NULL if due to error it has no leaf to return. Make its callers to check the returned leaf pointer, and release the leaf if it is not NULL. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes openzfs#12366 Closes openzfs#16159
behlendorf
pushed a commit
that referenced
this pull request
May 29, 2024
Depending on kind of error zap_expand_leaf() may return with or without valid leaf reference held. Make sure it returns NULL if due to error it has no leaf to return. Make its callers to check the returned leaf pointer, and release the leaf if it is not NULL. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes #12366 Closes #16159
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Sep 4, 2024
Depending on kind of error zap_expand_leaf() may return with or without valid leaf reference held. Make sure it returns NULL if due to error it has no leaf to return. Make its callers to check the returned leaf pointer, and release the leaf if it is not NULL. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes openzfs#12366 Closes openzfs#16159
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.
Depending on kind of error zap_expand_leaf() may return with or without valid leaf reference held. Make sure it returns NULL if due to error it has no leaf to return. Make its callers to check the returned leaf pointer, and release the leaf if it is not NULL.
Closes #12366
How Has This Been Tested?
Not really.
Types of changes
Checklist:
Signed-off-by.