Make IoctlFlags robust to unknown flags #62
Conversation
|
It looks good to me, and I guess new users of this crate would want to enable it by default. So, can you add a doc comment to the feature to recommend this feature enabled at least? |
|
I would prefer to increment the version to 0.8.0, which semver indicates is a breaking change, rather than gate this behind a feature that might be difficult to discover, since I expect that users want this change and, as you said, the previous error is impossible to fix without changing the -sys source. |
Returning Error::UnrecognizedIoctls is not helpful for developers because the only way to solve it is modifying this userfaultfd-rs crate implementation. For example, Linux v6.6 adds UFFDIO_POISON bit to uffdio_register.iotcls. With that change, Uffd::register() always fails even if the user don't care about the new UFFDIO_POISON feature. Using an unnamed flag for externally defined flag is originally recommended. https://docs.rs/bitflags/latest/bitflags/#externally-defined-flags Since IoctlFlags accepts any unknown flags, Error::UnrecognizedIoctls is needless and removed.
|
Thank you for reviewing. Updated the commit. It also removes |
keiichiw
left a comment
keiichiw
left a comment
There was a problem hiding this comment.
LGTM.
Although it'd be optional, isn't it a good timing to clean up and update CHANGELOG.md to mention this breaking change?
|
I will update the CHANGELOG in another PR. |
|
Created #63 for changelog update. |
|
Published 0.8.0 |
UFFD support is not compatible with newer kernels as there was a new ioctl option added that the older userfaultfd version does not recognize:
```
[PUT /snapshot/load][400] loadSnapshotBadRequest &{FaultMessage:Load snapshot error: Failed to restore from snapshot: Failed to load guest memory: Error creating guest memory from uffd: Failed to register memory address range with the userfaultfd object: Unrecognized ioctl flags: 284}
```
This was reported in bytecodealliance/userfaultfd-rs#61 and fixed in bytecodealliance/userfaultfd-rs#62
UFFD support is not compatible with newer kernels as there was a new ioctl option added that the older userfaultfd version does not recognize:
```
[PUT /snapshot/load][400] loadSnapshotBadRequest &{FaultMessage:Load snapshot error: Failed to restore from snapshot: Failed to load guest memory: Error creating guest memory from uffd: Failed to register memory address range with the userfaultfd object: Unrecognized ioctl flags: 284}
```
This was reported in bytecodealliance/userfaultfd-rs#61 and fixed in bytecodealliance/userfaultfd-rs#62
Signed-off-by: Vadim Berezniker <[email protected]>
3 participants
This is the mitigation for #61.
Returning Error::UnrecognizedIoctls is inconvinient for developers because the only way to solve it is modifying this userfaultfd-rs crate implementation.
For example, Linux v6.6 adds UFFDIO_POISON bit to
uffdio_register.iotcls. With that change, Uffd::register() always fails even if the user don't care about the new UFFDIO_POISON feature.
We need to guard this change behind the ignore_unknown_ioctlflags feature because Error::UnrecognizedIoctls is already published and some user may depend on the error.
Using an unnamed flag for externally defined flag is originally recommended.
https://docs.rs/bitflags/latest/bitflags/#externally-defined-flags