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

Workaround UBSAN errors for variable arrays #15510

Merged
merged 1 commit into from Nov 13, 2023

Conversation

tonyhutter
Copy link
Contributor

Motivation and Context

Workaround #15145

Description

This gets around UBSAN errors when using arrays at the end of structs. It converts some zero-length arrays to variable length arrays and disables UBSAN checking on certain modules.

It is based off of the patch from #15460.

How Has This Been Tested?

Reproduced original issue on Ubuntu 23.10 (which has UBSAN enabled by default). Applied patch, did a module load and import and saw no UBSAN errors.

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:

This gets around UBSAN errors when using arrays at the end of
structs.  It converts some zero-length arrays to variable length
arrays and disables UBSAN checking on certain modules.

It is based off of the patch from openzfs#15460.

Addresses: openzfs#15145
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Co-authored-by: Tony Hutter <hutter2@llnl.gov>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
@tonyhutter
Copy link
Contributor Author

We really need a test suite run with UBSAN enabled (like Ubuntu 23.10) to test this before it gets merged.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Nov 10, 2023
@ThomasLamprecht
Copy link
Contributor

ThomasLamprecht commented Nov 11, 2023

I applied this on our kernel with UBSAN_BOUNDS checking enabled again, then booted that on a test-host with ZFS on root, I could not see the oopses again that we observed on that exact platform previously.

So, consider this:
Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

ProxBot pushed a commit to proxmox/pve-kernel that referenced this pull request Nov 12, 2023
We have a slightly better fix where only a few targeted ZFS module
parts are added to the UBSAN ignore-list, so the rest of the kernel
still gets exposure.

Link: openzfs/zfs#15510
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 13, 2023
@behlendorf behlendorf merged commit 786641d into openzfs:master Nov 13, 2023
21 of 25 checks passed
@behlendorf behlendorf added this to To do in 2.2-release Nov 13, 2023
@tonyhutter tonyhutter moved this from To do to In progress in 2.2-release Nov 13, 2023
ixhamza pushed a commit to truenas/zfs that referenced this pull request Nov 20, 2023
This gets around UBSAN errors when using arrays at the end of
structs.  It converts some zero-length arrays to variable length
arrays and disables UBSAN checking on certain modules.

It is based off of the patch from openzfs#15460.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Issue openzfs#15145
Closes openzfs#15510
@behlendorf behlendorf moved this from In progress to Done in 2.2-release Nov 27, 2023
jiangcuo pushed a commit to jiangcuo/zfsonlinux that referenced this pull request Dec 2, 2023
Link: openzfs/zfs#15510
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
This gets around UBSAN errors when using arrays at the end of
structs.  It converts some zero-length arrays to variable length
arrays and disables UBSAN checking on certain modules.

It is based off of the patch from openzfs#15460.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Issue openzfs#15145
Closes openzfs#15510
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jan 26, 2024
This gets around UBSAN errors when using arrays at the end of
structs.  It converts some zero-length arrays to variable length
arrays and disables UBSAN checking on certain modules.

It is based off of the patch from openzfs#15460.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Issue openzfs#15145
Closes openzfs#15510
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jan 29, 2024
This gets around UBSAN errors when using arrays at the end of
structs.  It converts some zero-length arrays to variable length
arrays and disables UBSAN checking on certain modules.

It is based off of the patch from openzfs#15460.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Issue openzfs#15145
Closes openzfs#15510
behlendorf pushed a commit that referenced this pull request Feb 5, 2024
This gets around UBSAN errors when using arrays at the end of
structs.  It converts some zero-length arrays to variable length
arrays and disables UBSAN checking on certain modules.

It is based off of the patch from #15460.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Issue #15145
Closes #15510
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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants