Dev #1053
Dev #1053
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
Thanks! |
|
Pleas squash first and second commits, otherwise build is broken in between. |
|
The two commits squashed. As for your other comment, will push it soon. |
|
Re/ generated files - my git status after make generate is |
|
https://github.com/google/syzkaller/blob/master/docs/contributing.md should answer your question (or we need to improve them). |
|
Done. I missed the note in the second link. |
That's because I just added it :) |
|
There are few more things to fix. I can merge this and then fix up myself. Not sure how much you are up to massaging this more, learning curve may be a bit steep for a first commit.
But we still need your CLA to merge: |
Done
I tried changing arch and re-run but compiler fails due to missing files. Any guide for this that you added a few minutes ago?..
Your guide says go 1.8 :) I upgraded since syzkaller refused to build, thought I'd go for the latest :/
|
No last minute guide this time :)
Will fix. Thanks.
Oh, that's corp CLA. I don't yet have any experience with this. Let's wait few more days because I won't be able to do anything other then also mailing them. |
This should be fixed by: |
|
I will try to switch to Go 1.12 now. So don't install 1.10 yet. |
Codecov Report
@@ Coverage Diff @@
## master #1053 +/- ##
=========================================
+ Coverage 67.15% 67.8% +0.65%
=========================================
Files 117 117
Lines 20568 20354 -214
=========================================
- Hits 13812 13802 -10
+ Misses 6103 5915 -188
+ Partials 653 637 -16
Continue to review full report at Codecov.
|
|
I rebased and re-pushed, now I see we have conflicting files. |
Please post full log. I can't understand what produces this output and what arch fails. |
And what exactly you run. |
|
Normally I'm running this: Today I tried replacing amd64 with 386/arm64 which caused the above failures. Running make extract is not pretty: GOOS=linux GOARCH=amd64 go build "-ldflags=-s -w -X github.com/google/syzkaller/sys.GitRevision=4d04a100e0f1e3e88840fdb253762556daf6a710+" -o bin/syz-extract ./sys/syz-extract extracting from aio.txt extracting from apparmor.txt extracting from ashmem.txt extracting from binder.txt extracting from binfmt.txt extracting from block.txt extracting from bpf.txt extracting from cdrom.txt extracting from cgroup.txt extracting from dri.txt extracting from filesystem.txt extracting from floppy.txt extracting from fs_btrfs.txt extracting from fs_ext4.txt extracting from fs_ioctl.txt extracting from fuse.txt extracting from i2c.txt extracting from input.txt extracting from ion.txt extracting from ipc.txt extracting from ipvs.txt extracting from key.txt extracting from kvm.txt extracting from loop.txt extracting from midi.txt extracting from namespaces.txt extracting from nbd.txt extracting from netfilter.txt extracting from netfilter_arp.txt extracting from netfilter_bridge.txt extracting from netfilter_ipv4.txt extracting from netfilter_ipv6.txt extracting from netfilter_targets.txt extracting from perf.txt extracting from prctl.txt extracting from random.txt extracting from rdma.txt extracting from rdma_cm.txt extracting from rtc.txt extracting from selinux.txt extracting from sg.txt extracting from smack.txt extracting from sndcontrol.txt extracting from sndseq.txt extracting from sndtimer.txt extracting from socket.txt extracting from socket_alg.txt extracting from socket_ax25.txt extracting from socket_bluetooth.txt extracting from socket_can.txt extracting from socket_inet.txt extracting from socket_inet6.txt extracting from socket_inet_dccp.txt extracting from socket_inet_icmp.txt extracting from socket_inet_sctp.txt extracting from socket_inet_tcp.txt extracting from socket_inet_udp.txt extracting from socket_ipx.txt extracting from socket_kcm.txt extracting from socket_key.txt extracting from socket_llc.txt extracting from socket_netlink.txt extracting from socket_netlink_crypto.txt extracting from socket_netlink_generic.txt extracting from socket_netlink_generic_fou.txt extracting from socket_netlink_generic_team.txt extracting from socket_netlink_netfilter.txt extracting from socket_netlink_route.txt extracting from socket_netlink_route_sched.txt extracting from socket_netlink_xfrm.txt extracting from socket_netrom.txt extracting from socket_nfc.txt extracting from socket_packet.txt extracting from socket_pppox.txt extracting from socket_rds.txt extracting from socket_tipc.txt extracting from socket_tipc_netlink.txt extracting from socket_unix.txt extracting from socket_vnet.txt extracting from socket_xdp.txt extracting from sr.txt extracting from sys.txt extracting from tty.txt extracting from tun.txt extracting from uffd.txt extracting from uhid.txt extracting from uinput.txt extracting from userio.txt extracting from vnet.txt extracting from xattr.txt generating linux/amd64... extracting from loop.txt generating linux/arm... generating linux/arm64... generating linux/ppc64le... Makefile:171: recipe for target 'extract' failed |
|
Yes, this is not supposed to work, see: |
|
So how do we proceed from here? |
////////////
Just run: |
|
The long error-ish output above is the result of make extract. |
|
My bad. I stopped reading carefully after several mentions of syz-extract. Welcome to C world. Nobody can build anything... If nothing works, as I said I can merge it as is and then fix up in a separate commit. |
Interesting enough:
Thanks for all your help! |
|
A final addition: I rebased to take your golang changes and now: but If you have any suggestions, I'll try them next week. Else, I'll dig into this. |
I think you need to add $GOPATH/bin to $PATH (that's a common setup because go get installs binaries to $GOPATH/bin). If you did not set $GOPATH, then it defaults to ~/go, so that will be ~/go/bin. |
|
The dev_kvm.txt for amd64 was different and should be fixed by: |
|
|
Updates:
|
|
But you pre-bootstrapped arm build in /images/linux before running |
sorry, I'm not familiar with this. Do you have a guide? |
What's the current status of running |
I cut some of it for readability's sake. |
|
This suggests that you are using an old kernel checkout. Extract need to run on the latest checkout. Older revisions will naturally break due to missing syscalls/consts/etc. Somewhere above I provided a commit that worked for me, take at least that commit. |
|
We're running syzkaller on https://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma but I changed it to Linus's master for now. |
Great!
If everybody use different trees on different revisions, it can't work unfortunately. Consider, you add some consts that are present in your tree, then somebody else adds own but removes yours (they are not in their tree), and so on. We use the latest upstream as the source of truth. I've also sent an internal email re CLA. Sorry that it's all so painful. The second time it really should be just: hopefully... :) |
Understood. We're usually rebased upstream as well and should catch up with Linus today or tomorrow.
Thanks for that, your help is appreciated.
Hopefully :) |
sys/linux/rdma.txt
Outdated
| } | ||
|
|
||
| mlx5_get_context_cmd { | ||
| command const[0x0, int32] |
There was a problem hiding this comment.
Are there any literal consts for these? Is it IB_USER_VERBS_CMD_GET_CONTEXT? Please use literal consts.
It's hard for me to find any traces of this in kernel source to do any checking or understand what happens.
Usually if I see, say, mlx5_get_context_cmd I can do https://elixir.bootlin.com/linux/v5.1-rc1/ident/mlx5_get_context_cmd and find corresponding kernel code. But in this case I find nothing.
While https://elixir.bootlin.com/linux/v5.1-rc1/ident/IB_USER_VERBS_CMD_GET_CONTEXT gives something.
sys/linux/rdma.txt
Outdated
| provider_in_words const[0x0, int16] | ||
| provider_out_words const[0x8, int16] | ||
| reserved const[0x0, int32] | ||
| comp_mask const[0x0, int32] |
There was a problem hiding this comment.
It seems these reserved and comp_mask fields correspond to ib_uverbs_ex_query_device struct:
https://elixir.bootlin.com/linux/v5.1-rc1/source/include/uapi/rdma/ib_user_verbs.h#L219
Please declare this struct explicitly with the name matching kernel definition.
It makes working with these description way easier. And in future we will have static checking for typos and bugs in descriptions:
#590
sys/linux/rdma.txt
Outdated
| # context | ||
| # =========== | ||
|
|
||
| mlx5_get_context_cmd_resp { |
There was a problem hiding this comment.
The first 2 fields seem to correspond to ib_uverbs_get_context_resp struct. It would be useful to have ib_uverbs_get_context_resp struct explicitly defined and used here. Something along the lines of:
type ib_uverbs_get_context_resp[DRIVER_DATA] {
async_fd int32
num_comp_vectors int32
driver_data DRIVER_DATA
}
mlx5_get_context_cmd_resp {
qp_tab_size int32
...
}
And then use ib_uverbs_get_context_resp[mlx5_get_context_cmd_resp] instantiation for mlx5.
Is there a kernel name for mlx5_get_context_cmd_resp? I can't find it in kernel:
https://elixir.bootlin.com/linux/v5.1-rc1/ident/mlx5_get_context_cmd_resp
If so please use it.
sys/linux/rdma.txt
Outdated
| # =========== | ||
|
|
||
| mlx5_get_context_cmd_resp { | ||
| async_fd int32 |
There was a problem hiding this comment.
Is it fd? If so use fd type. If it's an fd of a particular subtype, e.g. event fd, then use fd_event.
sys/linux/rdma.txt
Outdated
| } | ||
|
|
||
| query_device_cmd_ex { | ||
| command const[IB_USER_VERBS_EX_CMD_QUERY_DEVICE, int32] |
There was a problem hiding this comment.
The first 6 fields seem to be duplicated across all requests. I think we need some templates for this and also match names with kernel names. Something along the lines of (if I correctly understand what happens here):
type ib_uverbs_cmd_hdr[CMD, REQ] {
command const[CMD, int32]
in_words bytesize4[req, int16]
out_words const[0, int16]
req REQ
}
type ib_uverbs_ex_cmd_hdr[CMD, REQ, RESP, PROVIDER_REQ, PROVIDER_RESP] {
command const[CMD, int32]
in_words bytesize4[request, int16]
out_words bytesize4[response, int16]
response ptr64[out, RESP]
provider_in_words bytesize4[provider_req, int16]
provider_out_words bytesize4[provider_resp, int16]
__u32 cmd_hdr_reserved;
request REQ
# or where/how they go?
provider_req PROVIDER_REQ
provider_resp ptr64[out, PROVIDER_RESP]
}
And then this can be used create actual requests as ib_uverbs_cmd_hdr[IB_USER_VERBS_EX_CMD_QUERY_DEVICE, ib_uverbs_ex_query_device] or ib_uverbs_ex_cmd_hdr[IB_USER_VERBS_EX_CMD_QUERY_DEVICE, ib_uverbs_ex_query_device, resp, ...]. Type voidmay be useful if there are cases where some of these things is not present,void` have static size of 0.
sys/linux/rdma.txt
Outdated
| ioctl$READ_COUNTERS(fd fd_rdma, cmd const[RDMA_VERBS_IOCTL], arg ptr[inout, ib_uverbs_read_counters_cmd]) | ||
|
|
||
| # device | ||
| syz_open_dev$ibv_device(dev ptr[in, string["/dev/infiniband/uverbs0"]], id intptr, flags flags[rdma_dev_open_flags]) fd_rdma |
There was a problem hiding this comment.
The only point in using syz_open_dev is that it can format dynamic strings (while we don't have direct support for this in descriptions syntax).
E.g. if you pass string "/dev/infiniband/uverbs#" as file name, then it will generate uverbs0, uverbs1, uverbs2, etc depending on id argument.
But since we don't use # syz_open_dev only leads to more complex reproducers.
Please use openat syscall to open the file.
sys/linux/rdma.txt
Outdated
| resource vcontext_handle[int32] | ||
|
|
||
| ib_uverbs_create_counters_cmd { | ||
| length bytesize8[ib_uverbs_create_counters_cmd, int16] |
There was a problem hiding this comment.
It seems all common fields for all ioctl's correspond to ib_uverbs_ioctl_hdr struct.
Please define it explicitly using templates similarly to the commands for write.
sys/linux/rdma.txt
Outdated
| syz_open_dev$ibv_device(dev ptr[in, string["/dev/infiniband/uverbs0"]], id intptr, flags flags[rdma_dev_open_flags]) fd_rdma | ||
| write$MLX5_GET_CONTEXT(fd fd_rdma, buf ptr[inout, mlx5_get_context_cmd], len len[buf]) | ||
| close$ibv_device(fd fd_rdma) | ||
| write$QUERY_DEVICE_EX(fd fd_rdma, buf ptr[inout, query_device_cmd_ex], len len[buf]) |
There was a problem hiding this comment.
The query_device_cmd_ex struct itself is only input, right?
There is ptr[out] for response and that response is output, right?
If so, use in direction here and for other write's. ptr[out] for response will override it for the response, so it still will be output.
sys/linux/rdma.txt
Outdated
| resource flow_handle[int32] | ||
|
|
||
| # defines | ||
| define IB_USER_VERBS_EX_CMD_QUERY_DEVICE 0x80000001 |
There was a problem hiding this comment.
This is defined in a header. Remove.
sys/linux/rdma.txt
Outdated
| driver_id const[RDMA_DRIVER_MLX5, int32] | ||
| reserved1 const[0x0, int32] | ||
|
|
||
| attr_id0 const[UVERBS_ATTR_READ_COUNTERS_HANDLE, int16] |
There was a problem hiding this comment.
We could make this an actual array:
attrs array[....]
and then use len[attars] for num_attrs.
This would allow fuzzer to generate more interesting options.
|
I've made a first pass over the actual descriptions. The common themes:
|
sys/linux/rdma.txt
Outdated
| ioctl$READ_COUNTERS(fd fd_rdma, cmd const[RDMA_VERBS_IOCTL], arg ptr[inout, ib_uverbs_read_counters_cmd]) | ||
|
|
||
| # device | ||
| syz_open_dev$ibv_device(dev ptr[in, string["/dev/infiniband/uverbs0"]], id intptr, flags flags[rdma_dev_open_flags]) fd_rdma |
There was a problem hiding this comment.
We now prefix all files that work with files in /dev with dev_ prefix.
And we have descirptions for /dev/infiniband/rdma_cm in dev_infiniband_rdma_cm.txt.
I think this should be named dev_infiniband_uverbs.txt
|
I think we sorted the CLA issue, is it possible to verify? |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Initial description of the kernel's RDMA subsystem. This patch covers most of the older write() interface as well as the some ioctl functions. Also disable rdma_cm's ib_qp_type flags as it conflicts with rdma's definition, and rdma builds first. Signed-off-by: Noa Osherovich <[email protected]>
Signed-off-by: Noa Osherovich <[email protected]>
|
As per offline discussion: we merge this now with some remaining TODOs, which may be resolved later incrementally. Thanks for bearing with me! I am merging this now. |
|
One last question: are there also some configs/sysclts that we need to enable to cover this? |
|
You'll need hardware / emulator for this. This also requires configuring Syzkaller to use another network device. |
These patches allow syzkaller to exercise the RDMA subsystem:
The first patch adds the needed descriptions and disables QP types in rdma_cm.
The second one updates CONTRIBUTORS as requested.
Noa