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

executor: linux: chroot into tmpfs with sandbox=none #4959

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

ramosian-glider
Copy link
Member

To prevent the executor from accidentally making the whole root file system immutable (which breaks fuzzing), modify sandbox=none to create a tmpfs mount and chroot into it before executing a program.

Fixes #4939.


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


@dvyukov
Copy link
Collaborator

dvyukov commented Jul 1, 2024

Please check how many syscalls got enabled with sandbox=none before/after this change with syzbot kernel.
I afraid that if we forget to mount something important in chroot, some subset of syscalls may end up being disabled forever. There may also be some other unexpected implications of this.

@dvyukov
Copy link
Collaborator

dvyukov commented Jul 1, 2024

I assume we can not re-enable EXT4_IOC_SHUTDOWN and EXT4_IOC_RESIZE_FS in sys/linux/init.go. If so, please add a commit that does it.

@ramosian-glider
Copy link
Member Author

On x86, the numbers of enabled syscalls with sandbox=none (3862) and sandbox=namespace (3492) didn't change.

@dvyukov
Copy link
Collaborator

dvyukov commented Jul 2, 2024

Also should be fixing #2933

@dvyukov
Copy link
Collaborator

dvyukov commented Jul 2, 2024

On x86, the numbers of enabled syscalls with sandbox=none (3862) and sandbox=namespace (3492) didn't change.

That's good!

@a-nogikh
Copy link
Collaborator

a-nogikh commented Jul 2, 2024

create a tmpfs mount and chroot into it before executing a program.

Nit: we set up sandboxes once per proc and those procs will execute many programs before they restart. But, as I understand, it will work fine anyway since it'll just restart the proc once the proc starts encountering problems with writing to the disk.

To prevent the executor from accidentally making the whole root file system
immutable (which breaks fuzzing), modify sandbox=none to create a tmpfs mount
and chroot into it before executing programs in a process.

According to `syz-manager -mode=smoke-test`, the number of enabled syscalls on
x86 doesn't change with this patch.

Fixes google#4939, google#2933, google#971.
@ramosian-glider
Copy link
Member Author

Updated the description according to your comments, added a patch to re-enable EXT4_IOC_SHUTDOWN and EXT4_IOC_RESIZE_FS.

sys/linux/init.go Outdated Show resolved Hide resolved
sys/linux/init.go Outdated Show resolved Hide resolved
sys/linux/init.go Outdated Show resolved Hide resolved
dvyukov
dvyukov previously approved these changes Jul 2, 2024
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.

Nice!

@dvyukov dvyukov enabled auto-merge July 2, 2024 08:34
Now that we chroot into tmpfs with sandbox=none, it should be safe to allow
using these ioctls, because they won't break the whole VM.

Update google#971.
These two constants are not used anywhere.
No functional change.
@dvyukov dvyukov added this pull request to the merge queue Jul 2, 2024
Merged via the queue into google:master with commit 07f0a0a Jul 2, 2024
16 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.

sys/linux: generating ioctl(FS_IOC_SETFLAGS) and ioctl(FS_IOC_FSSETXATTR) may break VM
3 participants