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

arm64: define the KASAN_SHADOW_SCALE_SHIFT macro #2518

Merged
merged 2 commits into from
Apr 6, 2023

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Mar 7, 2023

arm64 defines the KASAN_SHADOW_SCALE_SHIFT macro from Makefile instead of defining it in a header file as is the case for other architectures. This causes ClangParser not to find it and fail on arm64 systems with CONFIG_KASAN=y.

This resolves the issue by defining the macro manually via CFLAGS. The definition is taken from kernel's arch/arm64/Makefile and it depends on the running kernel configuration. Therefore, the first commit adds support for parsing the kernel config.

This fixes the runqlat.bt, tcpdrop.bt, and undump.bt tools on aarch64+debug kernel which previously failed with:

# /usr/share/bpftrace/tools/runqlat.bt 
/lib/modules/5.14.0-283.el9.aarch64+debug/source/arch/arm64/include/asm/memory.h:308:9: error: use of undeclared identifier 'KASAN_SHADOW_SCALE_SHIFT'
/lib/modules/5.14.0-283.el9.aarch64+debug/source/arch/arm64/include/asm/memory.h:308:9: error: use of undeclared identifier 'KASAN_SHADOW_SCALE_SHIFT'
/lib/modules/5.14.0-283.el9.aarch64+debug/source/arch/arm64/include/asm/memory.h:308:9: error: returning 'void' from a function with incompatible result type 'phys_addr_t' (aka 'unsigned long long')
Checklist
  • Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

@danobi
Copy link
Member

danobi commented Mar 17, 2023

It seems like kconfig values are stored in BTF: https://github.com/libbpf/libbpf/blob/b5e9722ec268702b2ec62863222277148e4fe656/src/libbpf.c#L3773-L3798

Is it possible for us to get it from BTF instead? I think it'll be more likely to be the source of truth, as reading from /boot/.. can suffer from corner cases when a new kernel is installed but the system hasn't rebooted yet.

config.emplace(option.substr(0, split), option.substr(split + 1));
}
}
gzclose(file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a break after this? If both locations are available, probably best not to duplicate effort.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, of course, thanks.

return;
config_locs = {
"/boot/config-" + std::string(utsname.release),
"/proc/config.gz",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe order procfs option first. The /boot/.. configs can suffer from skew when installing new kernels.

@viktormalik
Copy link
Contributor Author

It seems like kconfig values are stored in BTF: https://github.com/libbpf/libbpf/blob/b5e9722ec268702b2ec62863222277148e4fe656/src/libbpf.c#L3773-L3798

Is it possible for us to get it from BTF instead? I think it'll be more likely to be the source of truth, as reading from /boot/.. can suffer from corner cases when a new kernel is installed but the system hasn't rebooted yet.

I may be missing something, but IIUC, this is a libbpf CO-RE feature when it can pass values of kernel configs to a BPF program via external symbols. The values are loaded from kconfig files upon program load in the same way as I used here.

@danobi
Copy link
Member

danobi commented Apr 6, 2023

It seems like kconfig values are stored in BTF: https://github.com/libbpf/libbpf/blob/b5e9722ec268702b2ec62863222277148e4fe656/src/libbpf.c#L3773-L3798
Is it possible for us to get it from BTF instead? I think it'll be more likely to be the source of truth, as reading from /boot/.. can suffer from corner cases when a new kernel is installed but the system hasn't rebooted yet.

I may be missing something, but IIUC, this is a libbpf CO-RE feature when it can pass values of kernel configs to a BPF program via external symbols. The values are loaded from kconfig files upon program load in the same way as I used here.

Oh, ha. I did not dig that far. Please ignore me :)

Copy link
Member

@danobi danobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about delay. Missed the notification

TEST(utils, parse_kconfig)
{
char path[] = "/tmp/configXXXXXX";
int fd = mkstemp(path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to unlink this after use

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done and resolved the changelog conflict.

In future, it may (and will) be useful to have access to the running
kernel configuration, e.g. to add config-specific compilation options to
ClangParser.

This adds and fills a new map BPFtrace::kconfig that maps config options
to their values. Both the option name and the value are strings. The
configuration is parsed from one of two sources:
- /proc/config.gz
- /boot/config-$(uname -r)

For testing purposes, the config filename may be passed through the
BPFTRACE_KCONFIG_TEST env variable.
arm64 defines this macro from Makefile instead of defining it in a
header file as is the case for other architectures. Since we're not
running make, we need to define the macro manually via CFLAGS.

The value definition is taken from kernel's arch/arm64/Makefile and it
depends on the running kernel configuration.

This fixes the runqlat.bt tcpdrop.bt, and undump.bt tools on
aarch64+debug kernel which previously failed with:

    # /usr/share/bpftrace/tools/runqlat.bt
    [...]/source/arch/arm64/include/asm/memory.h:300:9: error: use of undeclared identifier 'KASAN_SHADOW_SCALE_SHIFT'
    [...]
@viktormalik viktormalik merged commit 4bcee64 into bpftrace:master Apr 6, 2023
@viktormalik viktormalik deleted the aarch-kasan-fix branch April 11, 2023 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants