Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove iov_iter_advance() from iter_read #11378

Merged
merged 1 commit into from
Dec 20, 2020

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

Issue #11375

Description

There's no need to call iov_iter_advance() in zpl_iter_read().
This was preserved from the previous code where it wasn't needed
but also didn't cause any problems. Now that the iter functions
also handle pipes that's no longer true. When fully reading a pipe
buffer iov_iter_advance() may result in the pipe buf release
function being called which will not be registered resulting in
a NULL dereference.

How Has This Been Tested?

The usual local ZTS run and xfstests which lightly exercise sendfile.

Since the issue was reported with pip I ran through installing
and uninstalling various packages to a home directory using ZFS.
No issues were observed and the installed packages worked
correctly in my testing.

For good measure I also checkout out the ZFS source and built
ZFS with gcc in a ZFS filesystem. No errors were reported and
the resulting kmods worked correctly. This was just a convenient
way to verify the correct behavior of a fairly wide range of utilities.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

There's no need to call iov_iter_advance() in zpl_iter_read().
This was preserved from the previous code where it wasn't needed
but also didn't cause any problems.  Now that the iter functions
also handle pipes that's no longer.  When fully reading a pipe
buffer iov_iter_advance() may results in the pipe buf release
function being called which will not be registered resulting in
a NULL dereference.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 19, 2020
@behlendorf behlendorf marked this pull request as ready for review December 20, 2020 01:54
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 20, 2020
@behlendorf behlendorf merged commit 9ac535e into openzfs:master Dec 20, 2020
behlendorf added a commit that referenced this pull request Dec 23, 2020
There's no need to call iov_iter_advance() in zpl_iter_read().
This was preserved from the previous code where it wasn't needed
but also didn't cause any problems.  Now that the iter functions
also handle pipes that's no longer the case.  When fully reading a
pipe buffer iov_iter_advance() may results in the pipe buf release
function being called which will not be registered resulting in
a NULL dereference.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #11375 
Closes #11378
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
There's no need to call iov_iter_advance() in zpl_iter_read().
This was preserved from the previous code where it wasn't needed
but also didn't cause any problems.  Now that the iter functions
also handle pipes that's no longer the case.  When fully reading a
pipe buffer iov_iter_advance() may results in the pipe buf release
function being called which will not be registered resulting in
a NULL dereference.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#11375 
Closes openzfs#11378
@behlendorf behlendorf deleted the issue-11375 branch April 19, 2021 19:20
rincebrain added a commit to rincebrain/zfs that referenced this pull request May 29, 2021
It seems like this should have gone with openzfs#11378.

Closes: openzfs#12041

Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
rincebrain added a commit to rincebrain/zfs that referenced this pull request May 30, 2021
It seems like this should have gone with openzfs#11378.

The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.

Closes: openzfs#12041

Suggested-by: @siebenmann
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
There's no need to call iov_iter_advance() in zpl_iter_read().
This was preserved from the previous code where it wasn't needed
but also didn't cause any problems.  Now that the iter functions
also handle pipes that's no longer the case.  When fully reading a
pipe buffer iov_iter_advance() may results in the pipe buf release
function being called which will not be registered resulting in
a NULL dereference.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#11375 
Closes openzfs#11378
behlendorf pushed a commit that referenced this pull request Jun 1, 2021
The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.  This will result in the following
warning being printed to the console as of the 5.12 kernel.

    Attempted to advance past end of bvec iter

This change should have been included with #11378 when a
similar change was made on the read side.

Suggested-by: @siebenmann
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Issue #11378
Closes #12041
Closes #12155
tonyhutter pushed a commit that referenced this pull request Jun 2, 2021
The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.  This will result in the following
warning being printed to the console as of the 5.12 kernel.

    Attempted to advance past end of bvec iter

This change should have been included with #11378 when a
similar change was made on the read side.

Suggested-by: @siebenmann
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Issue #11378
Closes #12041
Closes #12155
(cherry picked from commit 3f81aba)
Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jun 2, 2021
The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.  This will result in the following
warning being printed to the console as of the 5.12 kernel.

    Attempted to advance past end of bvec iter

This change should have been included with openzfs#11378 when a
similar change was made on the read side.

Suggested-by: @siebenmann
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Issue openzfs#11378
Closes openzfs#12041
Closes openzfs#12155
(cherry picked from commit 3f81aba)
Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jun 3, 2021
The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.  This will result in the following
warning being printed to the console as of the 5.12 kernel.

    Attempted to advance past end of bvec iter

This change should have been included with openzfs#11378 when a
similar change was made on the read side.

Suggested-by: @siebenmann
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Issue openzfs#11378
Closes openzfs#12041
Closes openzfs#12155
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jun 8, 2021
The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.  This will result in the following
warning being printed to the console as of the 5.12 kernel.

    Attempted to advance past end of bvec iter

This change should have been included with openzfs#11378 when a
similar change was made on the read side.

Suggested-by: @siebenmann
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Issue openzfs#11378
Closes openzfs#12041
Closes openzfs#12155
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jun 9, 2021
The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.  This will result in the following
warning being printed to the console as of the 5.12 kernel.

    Attempted to advance past end of bvec iter

This change should have been included with openzfs#11378 when a
similar change was made on the read side.

Suggested-by: @siebenmann
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Issue openzfs#11378
Closes openzfs#12041
Closes openzfs#12155
tonyhutter pushed a commit that referenced this pull request Jun 23, 2021
The additional iter advance is incorrect, as copy_from_iter() has
already done the right thing.  This will result in the following
warning being printed to the console as of the 5.12 kernel.

    Attempted to advance past end of bvec iter

This change should have been included with #11378 when a
similar change was made on the read side.

Suggested-by: @siebenmann
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Issue #11378
Closes #12041
Closes #12155
(cherry picked from commit 3f81aba)
Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant