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.

Sign up for GitHub

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

Jump to bottom

Fix integer overflow of ZTOI(zp)->i_generation #8858

Merged
merged 1 commit into from
Jun 6, 2019
Merged

Fix integer overflow of ZTOI(zp)->i_generation #8858

merged 1 commit into from
Jun 6, 2019

Conversation

tcaputi
Copy link
Contributor

@tcaputi tcaputi commented Jun 5, 2019

The ZFS on-disk format stores each inode's generation ID as a 64
bit number on disk and in-core. However, the Linux kernel's inode
is only a 32 bit number. In most places, the code handles this
correctly, but the cast is missing in zfs_rezget(). For many pools,
this isn't an issue since the generation ID is computed as the
current txg when the inode is created and many pools don't have
more than 2^32 txgs.

For the pools that have more txgs, this issue causes any inode with
a high enough generation number to report IO errors after a call to
"zfs rollback" while holding the file or directory open. This patch
simply adds the missing cast.

Signed-off-by: Tom Caputi tcaputi@datto.com

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

Fix integer overflow of ZTOI(zp)->i_generation
6740362 
The ZFS on-disk format stores each inode's generation ID as a 64
bit number on disk and in-core. However, the Linux kernel's inode
is only a 32 bit number. In most places, the code handles this
correctly, but the cast is missing in zfs_rezget(). For many pools,
this isn't an issue since the generation ID is computed as the
current txg when the inode is created and many pools don't have
more than 2^32 txgs.

For the pools that have more txgs, this issue causes any inode with
a high enough generation number to report IO errors after a call to
"zfs rollback" while holding the file or directory open. This patch
simply adds the missing cast.

Signed-off-by: Tom Caputi <tcaputi@datto.com>
@tcaputi
Copy link
Contributor Author

tcaputi commented Jun 5, 2019

This patch is a bit tricky to test because it requires a pool with txg > 2^32. Perhaps we can figure out a way to test this in the future.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jun 5, 2019
@codecov
Copy link

codecov bot commented Jun 6, 2019

Codecov Report

Merging #8858 into master will decrease coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8858      +/-   ##
==========================================
- Coverage    78.8%   78.68%   -0.12%     
==========================================
  Files         382      382              
  Lines      117771   117771              
==========================================
- Hits        92806    92671     -135     
- Misses      24965    25100     +135
Flag Coverage Δ
#kernel 79.36% <100%> (+0.02%) ⬆️
#user 67.21% <ø> (-0.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ecc202...6740362. Read the comment docs.

alek-p
alek-p approved these changes Jun 6, 2019
View reviewed changes
Copy link
Contributor

@alek-p alek-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jun 6, 2019
@behlendorf behlendorf merged commit 3ce85b5 into openzfs:master Jun 6, 2019
behlendorf pushed a commit that referenced this pull request Jun 7, 2019
@behlendorf
Fix integer overflow of ZTOI(zp)->i_generation
35050ef 
The ZFS on-disk format stores each inode's generation ID as a 64
bit number on disk and in-core. However, the Linux kernel's inode
is only a 32 bit number. In most places, the code handles this
correctly, but the cast is missing in zfs_rezget(). For many pools,
this isn't an issue since the generation ID is computed as the
current txg when the inode is created and many pools don't have
more than 2^32 txgs.

For the pools that have more txgs, this issue causes any inode with
a high enough generation number to report IO errors after a call to
"zfs rollback" while holding the file or directory open. This patch
simply adds the missing cast.

Reviewed-by: Alek Pinchuk <apinchuk@datto.com>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes #8858
PaulZ-98 added a commit to datto/zfs that referenced this pull request Jul 1, 2019
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
0b567b7 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
@PaulZ-98 PaulZ-98 mentioned this pull request Jul 1, 2019
Merged
12 tasks
PaulZ-98 added a commit to datto/zfs that referenced this pull request Jul 2, 2019
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
87a5fc3 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
PaulZ-98 added a commit to datto/zfs that referenced this pull request Jul 2, 2019
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
a4cc039 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
PaulZ-98 added a commit to datto/zfs that referenced this pull request Jul 16, 2019
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
bc2e82e 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
PaulZ-98 added a commit to datto/zfs that referenced this pull request Jul 17, 2019
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
59bb422 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
behlendorf pushed a commit that referenced this pull request Jul 27, 2019
@PaulZ-98 @behlendorf
Develop tests for issues #5866 and #8858
693c1fc 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Issue #5866
Issue #8858 
Closes #8978
behlendorf added a commit to behlendorf/zfs that referenced this pull request Jul 29, 2019
@behlendorf
Revert "Develop tests for issues openzfs#5866 and openzfs#8858"
c148a5b 
This reverts commit 693c1fc.  This
change resulted in a kmem leak being observed in existing code which
needs to be indentified and addressed.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
behlendorf added a commit that referenced this pull request Jul 29, 2019
@behlendorf
Revert "Develop tests for issues #5866 and #8858"
1e620c9 
This reverts commit 693c1fc.  This
change resulted in a kmem leak being observed in existing code which
needs to be identified and addressed.

Reviewed-by: Paul Zuchowski <pzuchowski@datto.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #8978
Closes #9090
allanjude pushed a commit to allanjude/zfs that referenced this pull request Aug 12, 2019
@PaulZ-98 @allanjude
Develop tests for issues openzfs#5866 and openzfs#8858
b3159d6 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Issue openzfs#5866
Issue openzfs#8858
Closes openzfs#8978
allanjude pushed a commit to allanjude/zfs that referenced this pull request Aug 12, 2019
@behlendorf @allanjude
Revert "Develop tests for issues openzfs#5866 and openzfs#8858"
9aaab95 
This reverts commit 693c1fc.  This
change resulted in a kmem leak being observed in existing code which
needs to be identified and addressed.

Reviewed-by: Paul Zuchowski <pzuchowski@datto.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#8978
Closes openzfs#9090
PaulZ-98 added a commit to datto/zfs that referenced this pull request Sep 24, 2019
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
b048293 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
allanjude pushed a commit to allanjude/zfs that referenced this pull request Oct 13, 2019
@PaulZ-98 @allanjude
Develop tests for issues openzfs#5866 and openzfs#8858
43a23cc 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Issue openzfs#5866
Issue openzfs#8858
Closes openzfs#8978
allanjude pushed a commit to allanjude/zfs that referenced this pull request Oct 13, 2019
@behlendorf @allanjude
Revert "Develop tests for issues openzfs#5866 and openzfs#8858"
b2f843f 
This reverts commit 693c1fc.  This
change resulted in a kmem leak being observed in existing code which
needs to be identified and addressed.

Reviewed-by: Paul Zuchowski <pzuchowski@datto.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#8978
Closes openzfs#9090
PaulZ-98 added a commit to datto/zfs that referenced this pull request Mar 30, 2020
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
721d68c 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
@PaulZ-98 PaulZ-98 mentioned this pull request Mar 30, 2020
Open
12 tasks
PaulZ-98 added a commit to datto/zfs that referenced this pull request Apr 6, 2020
@PaulZ-98
Develop tests for issues openzfs#5866 and openzfs#8858
b2518fc 
Provide zfstest coverage for these two issues which
were a panic accessing extended attributes and
a problem comparing 64 bit and 32 bit generation
numbers.

Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
@problame problame mentioned this pull request Sep 25, 2020
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@alek-p alek-p alek-p approved these changes

@gmelikov gmelikov gmelikov approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tcaputi @behlendorf @alek-p @gmelikov