Skip to content

Conversation

@ramosian-glider
Copy link
Member

As shown in #5565, SYZOS code in the guest section cannot reference global data, because it is relocated into the guest memory.

While arm64 executor has a dynamic check for data accesses, it is virtually impossible to do the same on x86 without implementing an x86 disassembler. Instead of doing so, introduce a build-time script that will detect instructions referencing global data on a best-effort basis.


Before sending a pull request, please review Contribution Guidelines:
https://github.com/google/syzkaller/blob/master/docs/contributing.md


@ramosian-glider
Copy link
Member Author

Example script output:

------------------------------------------------------------------
[FAIL] Found problematic data access instructions in 'guest'.
The following instructions are likely to cause crashes in SyzOS:
    1ea326:	48 8d 15 3b 26 04 00 	lea    0x4263b(%rip),%rdx        # 22c968 <_nl_C_name+0x208f>
------------------------------------------------------------------

@ramosian-glider
Copy link
Member Author

And

[OK] No problematic data access instructions found in 'guest'.

@a-nogikh
Copy link
Collaborator

I wonder if it'd be better to represent in as one more https://github.com/google/syzkaller/blob/master/executor/style_test.go test.

@ramosian-glider
Copy link
Member Author

style_test.go is checking the source code, whereas here we need to check the compiled binaries, do you think this case is a good fit?

@ramosian-glider ramosian-glider force-pushed the check-syzos branch 4 times, most recently from 47669cd to a59c85f Compare September 10, 2025 12:54
@a-nogikh
Copy link
Collaborator

style_test.go is checking the source code, whereas here we need to check the compiled binaries, do you think this case is a good fit?

Ah, right, you need a binary.

Technically, it's quite straightforward to build executor from Go and we do it from multiple tests, e.g.

executor := csource.BuildExecutor(t, target, "../..", "-fsanitize-coverage=trace-pc", "-g")

We also do have tests that build it for every arch (for which there's a compiler):

func TestExecutor(t *testing.T) {


How long does the test currently take? Is it necessarily better to do the check on each syzkaller build rather than make it a presubmit test?

@ramosian-glider
Copy link
Member Author

ramosian-glider commented Sep 11, 2025

Technically, it's quite straightforward to build executor from Go and we do it from multiple tests, e.g.

Note that the script does not build the executor, it looks at $TARGETOS and $TARGETARCH and takes the existing executor (the makefile target depends on it).
If we decide to rewrite the script in Go it will still have to parse objdump output and search for the patterns in it. Do you think Go is more preferable for this task?

We also do have tests that build it for every arch (for which there's a compiler):

For now SYZOS is only implemented for arm64 and amd64.

How long does the test currently take? Is it necessarily better to do the check on each syzkaller build rather than make it a presubmit test?

It takes 0.05 seconds, slightly slower than /bin/ls.
I think there is some value in running it for every build, because running the presubmits isn't something people do locally when changing the syzkaller.
This script adds an extra output line to the make invocation - if that will be bothering anyone we can change the script to only report failures.

@a-nogikh
Copy link
Collaborator

If we decide to rewrite the script in Go it will still have to parse objdump output and search for the patterns in it. Do you think Go is more preferable for this task?

If it'd be a presubmit test, I think it would be easier to orchestrate the process from Go. If we do it each build, probably doesn't matter much.

It takes 0.05 seconds, slightly slower than /bin/ls.

Cool!

This script adds an extra output line to the make invocation - if that will be bothering anyone we can change the script to only report failures.

If there's no much value in the extra output, I'd suggest we drop it right away.

Otherwise LGTM.

a-nogikh
a-nogikh previously approved these changes Sep 11, 2025
As shown in google#5565,
SYZOS code in the `guest` section cannot reference global data,
because it is relocated into the guest memory.

While arm64 executor has a dynamic check for data accesses, it is
virtually impossible to do the same on x86 without implementing an
x86 disassembler. Instead of doing so, introduce a build-time script
that will detect instructions referencing global data on a best-effort
basis.
Replace the switch statement in guest_handle_wr_crn() with a series of
if statements.
When compiling the executor in syz-env-old, -fstack-protector may
kick in and introduce global accesses that tools/check-syzos.sh reports.

To prevent this, introduce the __no_stack_protector macro attribute that
disable stack protection for the function in question, and use it for
guest code.

While at it, factor out some common definitions into common_kvm_syzos.h
@ramosian-glider
Copy link
Member Author

I changed the happy path to use echoerr instead of echo, so that make does not print anything by default.

@ramosian-glider ramosian-glider added this pull request to the merge queue Sep 11, 2025
Merged via the queue into google:master with commit 08b1234 Sep 11, 2025
28 of 33 checks passed
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.

2 participants