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

support CONFIG_RANDOMIZE_BASE=y #4828

Merged
merged 7 commits into from
Jul 3, 2024
Merged

Conversation

jiangenj
Copy link
Contributor

The current implementation doesn't work well when CONFIG_RANDOMIZE_BASE enabled.

Taken some arm64 devices for example:
kaslr_offset is diff at bits 12-40, and kernel modules are loaded at 2GB space,
so we have ffffffd342e10000 T _stext where uppper 32bit is ffffffd3. However,
if we check modules range, the 1st module is loaded at 0xffffffd2eeb2a000,
while the last module is loaded at 0xffffffd2f42c4000.
We can see the upper 32bits are diff for core kernel and modules.

If we use current 32bits for covered PC, we will get wrong module address
recovered, which uses pcBase from core kernel + lower 32bits offset.

So we need to move to 64bit cover and signal.

Besides, there are some other fixes and improvement in the PR to align with the 64bits support:

  • Fix wrong module size which should be .text size instead of that from /proc/modules.
  • Calculate kaslr_offset by subtracting _stext address on device and _stext from elf file.
  • Uniform Module and KernelModule which contains Name, Addr, Path, Size.

@jiangenj jiangenj requested a review from avagin as a code owner May 22, 2024 01:48
@jiangenj jiangenj force-pushed the 64bits-full branch 2 times, most recently from 847a622 to c05ba08 Compare May 22, 2024 06:33
@jiangenj jiangenj changed the title support CONFIG_RANDOMIZE_BASE support CONFIG_RANDOMIZE_BASE=y May 22, 2024
Copy link

codecov bot commented May 22, 2024

Codecov Report

Attention: Patch coverage is 14.93213% with 188 lines in your changes missing coverage. Please review.

Project coverage is 61.0%. Comparing base (757f06b) to head (2c57f85).

Current head 2c57f85 differs from pull request most recent head 7c70de3

Please upload reports for the commit 7c70de3 to get more accurate results.

Additional details and impacted files
Files Coverage Δ
pkg/cover/canonicalizer.go 93.7% <100.0%> (-3.9%) ⬇️
syz-manager/covfilter.go 23.6% <ø> (+23.6%) ⬆️
pkg/cover/backend/gvisor.go 18.0% <0.0%> (ø)
pkg/vminfo/netbsd.go 50.0% <0.0%> (ø)
pkg/vminfo/openbsd.go 42.9% <0.0%> (ø)
pkg/vminfo/vminfo.go 74.4% <50.0%> (ø)
syz-manager/cover.go 0.0% <0.0%> (ø)
pkg/cover/report.go 81.8% <50.0%> (-1.0%) ⬇️
pkg/cover/backend/mach-o.go 0.0% <0.0%> (ø)
pkg/cover/backend/backend.go 0.0% <0.0%> (ø)
... and 5 more

... and 54 files with indirect coverage changes

@dvyukov dvyukov requested review from dvyukov, a-nogikh and kalder and removed request for avagin May 31, 2024 08:03
@dvyukov
Copy link
Collaborator

dvyukov commented May 31, 2024

CC @eprucka3 @kalder since this touches Android modules support.

@dvyukov
Copy link
Collaborator

dvyukov commented May 31, 2024

CONFIG_RANDOMIZE_BASE does not work for kernel modules since we subtract vmlinux's kaslr_offset() from modules as well, right?

  • Always use KernelModule instead of ptr.

Why do we need this? This will increase size of all these objects that embed KernelModule by value.

@eprucka3
Copy link
Contributor

eprucka3 commented May 31, 2024

CONFIG_RANDOMIZE_BASE does not work for kernel modules since we subtract vmlinux's kaslr_offset() from modules as well, right?

There was a recent change to getModuleTextAddr, so the modules offsets are pulled directly from /sys/module which already includes the offset. This change included removing the kaslr offset in discoverModulesLinux.

@jiangenj
Copy link
Contributor Author

jiangenj commented Jun 4, 2024

CONFIG_RANDOMIZE_BASE does not work for kernel modules since we subtract vmlinux's kaslr_offset() from modules as well, right?

  • Always use KernelModule instead of ptr.

Why do we need this? This will increase size of all these objects that embed KernelModule by value.

Fixed to use ptr only

@jiangenj
Copy link
Contributor Author

jiangenj commented Jun 4, 2024

CONFIG_RANDOMIZE_BASE does not work for kernel modules since we subtract vmlinux's kaslr_offset() from modules as well, right?

There was a recent change to getModuleTextAddr, so the modules offsets are pulled directly from /sys/module which already includes the offset. This change included removing the kaslr offset in discoverModulesLinux.

Right, there are two kinds of offset for modules:

  1. for module loaded into randomized address after each reboot, which was already supported in canonicalizer.go.
  2. kaslr offset introduced by CONFIG_RANDOMIZE_BASE, which is presented in modules address in /proc/modules. The PR is to support removing the kaslr_offset from module address.

@dvyukov
Copy link
Collaborator

dvyukov commented Jun 10, 2024

CONFIG_RANDOMIZE_BASE does not work for kernel modules since we subtract vmlinux's kaslr_offset() from modules as well, right?

There was a recent change to getModuleTextAddr, so the modules offsets are pulled directly from /sys/module which already includes the offset. This change included removing the kaslr offset in discoverModulesLinux.

Right, there are two kinds of offset for modules:

  1. for module loaded into randomized address after each reboot, which was already supported in canonicalizer.go.
  2. kaslr offset introduced by CONFIG_RANDOMIZE_BASE, which is presented in modules address in /proc/modules. The PR is to support removing the kaslr_offset from module address.

Do we remove kaslr offset from module address twice now?
How does it manifest? Do we have cove coverage reports for modules? Do they work?

Overall it's hard to move forward with you PRs because they mix lots of different fixes, large refactorings, and performance optimizations, and individual commit don't have any explanation.

For example this one:
3b985ce
if you send it in a separate PR, we can merge right away and remove it from the plate.

This one:
64f6208
It would be good to understand what's calling it repeatedly. Perhaps something should be fixed at the caller side. Currently it's wrong, if I call the function second time with different dirs, it will return the old result, also error is not cached. But generally such caching at the lowest level just tries to work around some architectural problems on higher levels.
But in this large PR all of this is lost among of other changes.

This commit:
fd2a127
Seems to be fixing something, but I can't understand what exactly.
If you send this fix as a separate PR with own explanation of the problem, and ideally a test (so that we don't break it tomorrow), then we could also merge it separately.

The KernelModule pointer refactorting also seems to be unrelated to the rest, I can't understand reasons behind it, and it just makes progressing with the rest of PR more difficult.

@jiangenj
Copy link
Contributor Author

let me see if I can split it into several smaller PRs.

@jiangenj
Copy link
Contributor Author

jiangenj commented Jul 1, 2024

@dvyukov updated, pls check again.

Copy link
Collaborator

@dvyukov dvyukov left a comment

Choose a reason for hiding this comment

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

Otherwise looks good to me.

Thanks for splitting other changes from this PR, it now looks much simpler.

pkg/rpcserver/rpcserver.go Outdated Show resolved Hide resolved
pkg/cover/backend/backend.go Outdated Show resolved Hide resolved
pkg/rpcserver/rpcserver.go Outdated Show resolved Hide resolved
@dvyukov dvyukov added this pull request to the merge queue Jul 3, 2024
Merged via the queue into google:master with commit d819d4d Jul 3, 2024
16 checks passed
@dvyukov
Copy link
Collaborator

dvyukov commented Jul 3, 2024

All gvisor instances started failing with:

failed to create rpc server: no symbol section

@dvyukov
Copy link
Collaborator

dvyukov commented Jul 3, 2024

Sent #4974

@@ -318,6 +318,10 @@ func generateReport(t *testing.T, target *targets.Target, test *Test) (*reports,
},
},
}
modules, err := backend.DiscoverModules(cfg.SysTarget, cfg.KernelObj, cfg.ModuleObj)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change broke syzkaller for gVisor:
2024/07/03 22:52:34 serving http on http://:8888
2024/07/03 22:52:34 [FATAL] failed to create rpc server: open vmlinux: no such file or directory

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

4 participants