create-diff-object: fix Clang UBSAN diff failure by ignoring unnamed data #1485
Conversation
…data This fixes a diff failure when generating livepatches for objects compiled with Clang and UBSAN (Undefined Behavior Sanitizer). When UBSAN is enabled, Clang heavily utilizes `.data..L__unnamed_XX` sections to store type descriptors and source location metadata. kpatch-build currently fails to handle these sections, resulting in errors like: fsnotify.o: changed section .data..L__unnamed_1 not selected for inclusion ERROR: fsnotify.o: 1 unsupported section change(s) create-diff-object: unreconcilable difference The numeric suffix (e.g., `_1`) in these section names is an unstable internal counter. If the patch modifies the code, these indices often shift, causing `create-diff-object` to incorrectly correlate unrelated UBSAN metadata between the original and patched objects. Since kpatch cannot support changes to existing data sections, the build aborts. Fix this by treating `.data..L__unnamed_` sections as unstable and uncorrelatable. This patch introduces `is_clang_unnamed_data()` to: 1. Explicitly skip these sections in `kpatch_correlate_sections()`. 2. Prevent `kpatch_correlate_static_local_variables()` from forcing correlation based on symbol usage. This ensures that UBSAN metadata sections are always marked as 'NEW' and allocated safely in the patch module, enabling support for livepatching UBSAN-instrumented kernels. Signed-off-by: Florent Revest <revest@chromium.org>
|
/packit test |
|
Thanks for triggering the tests @joe-lawrence! :) I can't seem to be able to open any logs for those "5 failing checks" - do you know if they are flakes or indicative of a real issue with my PR ? Also, if you have access to logs and they point to an issue in my PR, could you share them here so I get a chance to fix the issue ? |
Hi @FlorentRevest , the packit tests are relatively new so bear with us during the learning curve :) Can you access any of these links: https://github.com/dynup/kpatch/pull/1485/checks?check_run_id=56410963239 And then @Tomcat-42, can you take a look at the test results to see if there are any real failures, or if these are false positives. Thanks! |
Of course! :)
This one opens but doesn't contain much apart from a link to the next two This one also opens but also doesn't contain much apart from a link to the next one
This one fails to load on any device I tried with: "This site can’t be reached |
|
Hi @FlorentRevest, the jobs are failing because we're amid a migration to testing on linux-6.17/Fedora-43 (#1484), but there is a blocking issue on Fedora-43 composes (https://gitlab.com/testing-farm/artemis/-/merge_requests/1499).
This link will not open because it is from the RH internal ranch (don't ask me why). Ideally we should wait to test on the newer kernel version, but if this PR is urgent enough I can rollback the linux-6.2 configs so we can get this merged. |
Ah, I see, thanks for checking!
Eh, don't worry, I know the pain :)
I don't think this PR is urgent and needs to land in a hurry no, this can probably wait for your migration to be over. ;) However, even if we wait for this migration to happen before merging, I'd really appreciate a code review in the meantime - when someone gets a chance of course - because the risk for us to have to livepatch a UBSAN-sanitized compilation unit grows every day and if we have to use this patch as a downstream addition, some sort of review from someone who understands kpatch better than me would give us a lot more confidence that my patch is not entirely unreasonable! :) Thanks! |
The patch looks reasonable to me. I'm not heavily experienced with either clang or UBSAN, but I can generate such sections with a simple userspace prog: #include <stdio.h>
#include <limits.h>
int main() {
int large = INT_MAX;
int val = 10;
int result = large + val;
printf("Result: %d\n", result);
return 0;
}and then building with various # CONFIG_UBSAN_SIGNED_OVERFLOW: -fsanitize=signed-integer-overflow
$ clang -c main.c -o main.o -fsanitize=signed-integer-overflow -fdata-sections && \
readelf --wide --sections main.o | grep unnamed
[ 5] .rodata..L__unnamed_2 PROGBITS 0000000000000000 0000d0 00000a 00 A 0 0 8
[ 6] .data..L__unnamed_1 PROGBITS 0000000000000000 0000e0 000018 00 WA 0 0 16
[ 7] .rela.data..L__unnamed_1 RELA 0000000000000000 0002a0 000030 18 I 13 6 8
# CONFIG_UBSAN_TRAP: -fsanitize-trap=undefined
$ clang -c main.c -o main.o -fsanitize=undefined -fdata-sections && \
readelf --wide --sections main.o | grep unnamed
[ 5] .rodata..L__unnamed_2 PROGBITS 0000000000000000 0000d8 00000a 00 A 0 0 8
[ 6] .data..L__unnamed_1 PROGBITS 0000000000000000 0000f0 000018 00 WA 0 0 16
[ 7] .rela.data..L__unnamed_1 RELA 0000000000000000 0002b0 000030 18 I 13 6 8
# CONFIG_KASAN: -fsanitize=kernel-address
$ clang -c main.c -o main.o -fsanitize=kernel-address -fdata-sections && \
readelf --wide --sections main.o | grep unnamed
[11] .data.__unnamed_1 PROGBITS 0000000000000000 000110 000040 00 WA 0 0 16
[12] .rela.data.__unnamed_1 RELA 0000000000000000 000438 000048 18 I 22 11 8now I know the PR is targeted to the UBSAN options, but should we care about the non-local naming for If you like, we could probably add this to our unit test collection. The easiest way would be to build with And finally, I'll tag @liu-song-6 to possibly chime in here as I know he uses clang for LTO builds. Have you encountered these UBSAN related anonymous sections? Any reservations about this CDO patch? |
|
@joe-lawrence Thanks for the heads-up. This patch looks good to me. We haven't shipped livepatch for UBSAN enabled kernels, because our production kernels have it disabled. But we have seen similar CDO failures in testing. I can't remember for sure whether it was UBSAN or some other debug CONFIGs. |
Thanks a lot for looking into it! I really appreciate! :)
That's a very good point - I have not tried with KASAN so I didn't know about those sections but I agree, it looks like they should be taken care of in the same way as the local ones as part of this PR.
I agree some tests here would be great and I'd be happy to look into it. I'll let you know if I run into issues. I am just gonna leave for some holidays around LPC now so I won't be able to take care of this short term but let me circle back to this after LPC. |
4 participants
This fixes a diff failure when generating livepatches for objects compiled with Clang and UBSAN (Undefined Behavior Sanitizer).
When UBSAN is enabled, Clang heavily utilizes
.data..L__unnamed_XXsections to store type descriptors and source location metadata. kpatch-build currently fails to handle these sections, resulting in errors like:fsnotify.o: changed section .data..L__unnamed_1 not selected for inclusion
ERROR: fsnotify.o: 1 unsupported section change(s)
create-diff-object: unreconcilable difference
The numeric suffix (e.g.,
_1) in these section names is an unstable internal counter. If the patch modifies the code, these indices often shift, causingcreate-diff-objectto incorrectly correlate unrelated UBSAN metadata between the original and patched objects. Since kpatch cannot support changes to existing data sections, the build aborts.Fix this by treating
.data..L__unnamed_sections as unstable and uncorrelatable.This patch introduces
is_clang_unnamed_data()to:kpatch_correlate_sections().kpatch_correlate_static_local_variables()from forcing correlation based on symbol usage.This ensures that UBSAN metadata sections are always marked as 'NEW' and allocated safely in the patch module, enabling support for livepatching UBSAN-instrumented kernels.