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

FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}() #13882

Merged
merged 1 commit into from
Sep 14, 2022
Merged

FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}() #13882

merged 1 commit into from
Sep 14, 2022

Conversation

ryao
Copy link
Contributor

@ryao ryao commented Sep 13, 2022 •
edited
Loading

Motivation and Context

When reviewing #13875, I noticed that our FreeBSD code has an issue where it converts from int64_t to int when calling vnlru_free{,_vfsops}(). The result is that if the int64_t is 1 << 36, the int will be 0, since the low bits are 0. Even when some low bits are set, a value such as ((1 << 36) + 1) would truncate to 1, which is wrong.

Description

There is protection against this on 32-bit platforms, but on 64-bit platforms, there is no check to protect us, so we add a check.

How Has This Been Tested?

The buildbot is expected to test for us.

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:

@ryao ryao mentioned this pull request Sep 13, 2022
Merged
13 tasks
@ryao ryao requested a review from amotin September 13, 2022 15:21
@ryao
FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}()
8cccaf0 
When reviewing openzfs#13875, I noticed that our FreeBSD code has an issue
where it converts from `int64_t` to `int` when calling
`vnlru_free{,_vfsops}()`. The result is that if the int64_t is `1 <<
36`, the int will be 0, since the low bits are 0. Even when some low
bits are set, a value such as `((1 << 36) + 1)` would truncate to 1,
which is wrong.

There is protection against this on 32-bit platforms, but on 64-bit
platforms, there is no check to protect us, so we add a check.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
@ghost ghost added the Status: Accepted Ready to integrate (reviewed, tested) label Sep 13, 2022
@behlendorf behlendorf merged commit ccec88f into openzfs:master Sep 14, 2022
beren12 pushed a commit to beren12/zfs that referenced this pull request Sep 19, 2022
@ryao @beren12
FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}()
938703e 
When reviewing openzfs#13875, I noticed that our FreeBSD code has an issue
where it converts from `int64_t` to `int` when calling
`vnlru_free{,_vfsops}()`. The result is that if the int64_t is `1 <<
36`, the int will be 0, since the low bits are 0. Even when some low
bits are set, a value such as `((1 << 36) + 1)` would truncate to 1,
which is wrong.

There is protection against this on 32-bit platforms, but on 64-bit
platforms, there is no check to protect us, so we add a check.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13882
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 27, 2022
@ryao @andrewc12
FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}()
1f1f950 
When reviewing openzfs#13875, I noticed that our FreeBSD code has an issue
where it converts from `int64_t` to `int` when calling
`vnlru_free{,_vfsops}()`. The result is that if the int64_t is `1 <<
36`, the int will be 0, since the low bits are 0. Even when some low
bits are set, a value such as `((1 << 36) + 1)` would truncate to 1,
which is wrong.

There is protection against this on 32-bit platforms, but on 64-bit
platforms, there is no check to protect us, so we add a check.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13882
ghost pushed a commit to truenas/zfs that referenced this pull request Sep 28, 2022
@ryao
FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}()
04112cd 
When reviewing openzfs#13875, I noticed that our FreeBSD code has an issue
where it converts from `int64_t` to `int` when calling
`vnlru_free{,_vfsops}()`. The result is that if the int64_t is `1 <<
36`, the int will be 0, since the low bits are 0. Even when some low
bits are set, a value such as `((1 << 36) + 1)` would truncate to 1,
which is wrong.

There is protection against this on 32-bit platforms, but on 64-bit
platforms, there is no check to protect us, so we add a check.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13882
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 28, 2022
@ryao @tonyhutter
FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}()
55816c6 
When reviewing openzfs#13875, I noticed that our FreeBSD code has an issue
where it converts from `int64_t` to `int` when calling
`vnlru_free{,_vfsops}()`. The result is that if the int64_t is `1 <<
36`, the int will be 0, since the low bits are 0. Even when some low
bits are set, a value such as `((1 << 36) + 1)` would truncate to 1,
which is wrong.

There is protection against this on 32-bit platforms, but on 64-bit
platforms, there is no check to protect us, so we add a check.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13882
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
@ryao @andrewc12
FreeBSD: Fix integer conversion for vnlru_free{,_vfsops}()
e375350 
When reviewing openzfs#13875, I noticed that our FreeBSD code has an issue
where it converts from `int64_t` to `int` when calling
`vnlru_free{,_vfsops}()`. The result is that if the int64_t is `1 <<
36`, the int will be 0, since the low bits are 0. Even when some low
bits are set, a value such as `((1 << 36) + 1)` would truncate to 1,
which is wrong.

There is protection against this on 32-bit platforms, but on 64-bit
platforms, there is no check to protect us, so we add a check.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13882
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amotin amotin amotin 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.
3 participants
@ryao @amotin @behlendorf