Correctly handle errors from kern_path() in zfsctl_snapdir_vget() #7864
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
As a regular kernel function, kern_path() returns errors as negative errnos, such as -ELOOP. zfsctl_snapdir_vget() must convert these into the positive errnos used throughout the ZFS code when it returns them to other ZFS functions so that the ZFS code properly sees them as errors. Signed-off-by: Chris Siebenmann <cks.git01@cs.toronto.edu> Closes openzfs#7764
behlendorf
approved these changes
Sep 4, 2018
gmelikov
approved these changes
Sep 4, 2018
|
Thankx all. Amazing to see the open source world in action.... |
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 5, 2018
As a regular kernel function, kern_path() returns errors as negative errnos, such as -ELOOP. zfsctl_snapdir_vget() must convert these into the positive errnos used throughout the ZFS code when it returns them to other ZFS functions so that the ZFS code properly sees them as errors. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chris Siebenmann <cks.git01@cs.toronto.edu> Closes openzfs#7764 Closes openzfs#7864
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 5, 2018
As a regular kernel function, kern_path() returns errors as negative errnos, such as -ELOOP. zfsctl_snapdir_vget() must convert these into the positive errnos used throughout the ZFS code when it returns them to other ZFS functions so that the ZFS code properly sees them as errors. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chris Siebenmann <cks.git01@cs.toronto.edu> Closes openzfs#7764 Closes openzfs#7864
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
Fix #7764
zfsctl_snapdir_vget()callskern_path()and then returns any errors untouched, which is incorrect behavior. As a regular kernel function,kern_path()returns errors as negative errnos, such as-ELOOP, but ZFS code expects errors to be positive errnos, such asELOOP. When these untouched negative errnos are returned to other ZFS functions, the functions incorrectly handle the results; in the case of #7764, this causeszpl_fh_to_dentry()to turn the negative error result into a positive number, which the kernel (in the form ofexportfs_decode_fh()) considers to be a pointer and immediately panics on.Description
The fix is to negate the return value from
kern_path().How Has This Been Tested?
To test this fix, I built a Fedora 28 VM with the official 0.7.9 DKMS modules, verified that it paniced as a NFS server following the reproduction steps in #7764, and then hand-patched the DKMS source to negate the
kern_path()return value. With my patched and rebuilt DKMS modules, the Fedora 28 NFS server didn't panic and clients just got the expectedESTALEerrors.Types of changes
Checklist:
Signed-off-by.