Various errors from UBSAN #16278
Labels
Type: Defect
Incorrect behavior (e.g. crash, hang)
Comments
|
I am also seeing this on Fedora 40 with their 6.8.9-300.fc40.x86_64 kernel and the current ZFS git tip. The comments in the source code suggest that |
|
Looks to be the same issue as the last few times that false positives for variable length arrays have popped up. This time with sa_hdr_phys->sa_lengths and mzap_phys->mz_chunk. Simply changing '[1]' to '[]' in the headers clears up the ubsan warnings. |
|
FWIW, mechanically changing |
|
@robn good point, I hadn't thought of that. In any case, the ubsan warnings are false positives and folks can wait on a proper fix. I just spun up a guess on a throwaway system to confirm. I didn't mean to suggest that anyone else should go do that with real data. |
|
Just a quick note. Hardened kernels with
Trace will look something like this: |
|
If this UBSAN is a harmless false positive, we could consider something like this |
|
Similar Story here on Fedora 40 AMD64 (inside KVM Virtual Machine, on top of Proxmox VE and ZFS over ZVOL for the Data Storage, NOT the Fedora "/" though). Full |
|
What's odd is that we've already disabled UBSAN on zap_micro.c but maybe it's not being honored for some reason on 6.9+. There is a horrible workaround (below), but hopefully we can find a cleaner solution. diff --git a/include/sys/zap_impl.h b/include/sys/zap_impl.h
index 2959aa9b2..ca0854da3 100644
--- a/include/sys/zap_impl.h
+++ b/include/sys/zap_impl.h
@@ -61,7 +61,15 @@ typedef struct mzap_phys {
uint64_t mz_salt;
uint64_t mz_normflags;
uint64_t mz_pad[5];
- mzap_ent_phys_t mz_chunk[1];
+ union {
+ struct {
+ mzap_ent_phys_t _mz_chunk[1];
+ };
+ struct {
+ mzap_ent_phys_t _compiler_needs_this[0];
+ mzap_ent_phys_t mz_chunk[];
+ };
+ };
/* actually variable size depending on block size */
} mzap_phys_t; |
|
This seems to work: diff --git a/module/Kbuild.in b/module/Kbuild.in
index 9e44364b7..f270e4561 100644
--- a/module/Kbuild.in
+++ b/module/Kbuild.in
@@ -490,6 +490,7 @@ zfs-$(CONFIG_PPC64) += $(addprefix zfs/,$(ZFS_OBJS_PPC_PPC64))
UBSAN_SANITIZE_zap_leaf.o := n
UBSAN_SANITIZE_zap_micro.o := n
UBSAN_SANITIZE_sa.o := n
+UBSAN_SANITIZE_zfs/zap_micro.o = n
# Suppress incorrect warnings from versions of objtool which are not
# aware of x86 EVEX prefix instructions used for AVX512.I'll put together a PR and see how it holds up in ZTS. |
13 tasks
robn
pushed a commit
to robn/zfs
that referenced
this issue
Jul 17, 2024
You can use the UBSAN_SANITIZE_* Kbuild options to exclude certain kernel objects from the UBSAN checks. We previously excluded zap_micro.o with: UBSAN_SANITIZE_zap_micro.o := n For some reason that didn't work for the 6.9 kernel, which wants us to use: UBSAN_SANITIZE_zfs/zap_micro.o := n Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tony Hutter <hutter2@llnl.gov> Closes openzfs#16278 Closes openzfs#16330
calccrypto
pushed a commit
to hpc/zfs
that referenced
this issue
Jul 17, 2024
You can use the UBSAN_SANITIZE_* Kbuild options to exclude certain kernel objects from the UBSAN checks. We previously excluded zap_micro.o with: UBSAN_SANITIZE_zap_micro.o := n For some reason that didn't work for the 6.9 kernel, which wants us to use: UBSAN_SANITIZE_zfs/zap_micro.o := n Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tony Hutter <hutter2@llnl.gov> Closes openzfs#16278 Closes openzfs#16330
|
#16330 fixes this for |
|
To fix both, this should actually be: |
|
hi |
|
I am using Fedora 40 with (currently) kernel 6.9.9-200.fc40.x86_64, with what is currently the git tip development version (2.2.99-560_gaea42e137) and I see these sa.c assertions. I see them in three code locations: All for 'uint16_t [1]'. Two of the three are clearly accessing |
|
@siebenmann thanks. Can you walk me though how you are getting those assertions to trigger, and can you also give me the specific command you are using to build ZFS? |
|
Re-opening issue |
|
The first two warnings trigger during system boot as my pools are imported and filesystems mounted. The third triggered slightly later, when I logged in and started various programs as part of session startup, but it's possible it would trigger shortly after boot even if I didn't log in (I log in immediately after boot). I'm building ZFS from a git checkout of the development tree with: Then I 'dnf update' to the new RPMs and let DKMS build the modules. Generally I do this immediately before a kernel update and reboot. |
|
@siebenmann thanks I can reproduce it now with: truncate -s 100M file
sudo ./zpool create tank `pwd`/file
sudo cp -a /var/log /tank
< out of disk space >
dmesg@thwalker3 your fix worked for me: I'll put together a PR. |
13 tasks
tonyhutter
added a commit
to tonyhutter/zfs
that referenced
this issue
Jul 24, 2024
This is a follow-on to 156a641 that ignores UBSAN errors in sa.c. Thank you @thwalker3 for the fix. Original-patch-by: @thwalker3 Signed-off-by: Tony Hutter <hutter2@llnl.gov> Closes openzfs#16278 Closes openzfs#16330 Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
tonyhutter
added a commit
to tonyhutter/zfs
that referenced
this issue
Jul 24, 2024
This is a follow-on to 156a641 that ignores UBSAN errors in sa.c. Thank you @thwalker3 for the fix. Original-patch-by: @thwalker3 Signed-off-by: Tony Hutter <hutter2@llnl.gov> Closes openzfs#16278 Closes openzfs#16330 Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
tonyhutter
added a commit
that referenced
this issue
Aug 6, 2024
This is a follow-on to 156a641 that ignores UBSAN errors in sa.c. Thank you @thwalker3 for the fix. Original-patch-by: @thwalker3 Signed-off-by: Tony Hutter <hutter2@llnl.gov> Closes #16278 Closes #16330 Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this issue
Sep 4, 2024
You can use the UBSAN_SANITIZE_* Kbuild options to exclude certain kernel objects from the UBSAN checks. We previously excluded zap_micro.o with: UBSAN_SANITIZE_zap_micro.o := n For some reason that didn't work for the 6.9 kernel, which wants us to use: UBSAN_SANITIZE_zfs/zap_micro.o := n Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tony Hutter <hutter2@llnl.gov> Closes openzfs#16278 Closes openzfs#16330
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this issue
Sep 4, 2024
This is a follow-on to 156a641 that ignores UBSAN errors in sa.c. Thank you @thwalker3 for the fix. Original-patch-by: @thwalker3 Signed-off-by: Tony Hutter <hutter2@llnl.gov> Closes openzfs#16278 Closes openzfs#16330 Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
System information
Describe the problem you're observing
Various errors in dmesg
Describe how to reproduce the problem
Happens in normal operation
Include any warning/errors/backtraces from the system logs
The text was updated successfully, but these errors were encountered: