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

sys/linux: Add the descriptions for the mapper control device #4565

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

YangChenyuan
Copy link
Contributor

Add the descriptions for the CEC device, which is implemented in linux/drivers/md/dm-ioctl.c.

Structures are defined in include/uapi/linux/dm-ioctl.h.

These descriptions help to detect 2 new unique crashes for the mapper control device: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bd504bcfec41a503b32054da5472904b404341a4

This description is generated by our project KernelGPT, and we plan to merge all our generated descriptions for the missing drivers and sockets.

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.0%. Comparing base (96d142e) to head (45d7278).
Report is 24 commits behind head on master.

❗ Current head 45d7278 differs from pull request most recent head f235983. Consider uploading reports for the commit f235983 to get more accurate results

Additional details and impacted files

see 3 files with indirect coverage changes

Copy link
Collaborator

@a-nogikh a-nogikh left a comment

Choose a reason for hiding this comment

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

Interesting!
I've left some comments. With those fixes the descriptions would look like what they should ideally be.

Having just a list of int fields in a struct is of course much better than having no descriptions at all, but we can make fuzzing much more efficient by also considering what values each field should normally take. The fuzzer would anyway try to break these rules, but knowing what values are actually expected by the kernel helps it go much deeper into the code.

Could your tool be taught to do this kind of analysis automatically as well?

sys/linux/dev_dm_ctl.txt.const Outdated Show resolved Hide resolved
sys/linux/dev_dm_ctl.txt Outdated Show resolved Hide resolved
sys/linux/dev_dm_ctl.txt Show resolved Hide resolved
sys/linux/dev_dm_ctl.txt Outdated Show resolved Hide resolved
sys/linux/dev_dm_ctl.txt Outdated Show resolved Hide resolved
@YangChenyuan
Copy link
Contributor Author

Thanks for your interest!

I think such an analysis of the data structure could be included in our tools, as long as we provide several examples to let LLMs understand what kinds of analysis or labels should be added to the data structure.
Plus, this analysis could be orthogonal to our current framework, where we can add one more pass for more precise data structure analysis in the end.

Thank you very much for your advice! We will extend our tool with such feature soon :)

@a-nogikh
Copy link
Collaborator

Please note that the CI tests have begun to fail. You can reproduce the problem by e.g. running ./tools/syz-env go test ./prog -short:

$ ./tools/syz-env go test ./prog -short
gcr.io/syzkaller/env:latest
--- FAIL: TestTransitivelyEnabledCallsLinux (0.02s)
    resources_test.go:76: still must be able to create epoll fd with epoll_create1
--- FAIL: TestTransitivelyEnabledCalls (0.85s)
    --- FAIL: TestTransitivelyEnabledCalls/linux/s390x (0.01s)
        resources_test.go:53: some calls are disabled: 4373/4389
        resources_test.go:57: disabled ioctl$DM_DEV_CREATE: no syscalls can create resource dm_ctl_dev, enable some syscalls that can create it [ioctl$DM_DEV_ARM_POLL ioctl$DM_DEV_CREATE ioctl$DM_DEV_REMOVE ioctl$DM_DEV_RENAME ioctl$DM_DEV_SET_GEOMETRY ioctl$DM_DEV_STATUS ioctl$DM_DEV_SUSPEND ioctl$DM_DEV_WAIT ioctl$DM_GET_TARGET_VERSION ioctl$DM_LIST_VERSIONS ioctl$DM_TABLE_CLEAR ioctl$DM_TABLE_DEPS ioctl$DM_TABLE_LOAD ioctl$DM_TABLE_STATUS ioctl$DM_TARGET_MSG ioctl$DM_VERSION]

Since a dm_ctl_dev value is required by all ioctl$DM_* calls, syzkaller cannot imagine a way to get a dm_ctl_dev value in the first place. There's a circular dependency. At runtime, the tool will figure out a way to construct these structures (it sometimes just substitutes 0 instead of the resource), but our current description checks don't take that into account.

I've sent #4580, if it turns out to be a viable approach, we could just use resource dm_ctl_dev[int64, opt] here and all tests will pass.

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