sys/fuchsia: fidl method ordinals are wrong

[dvyukov]

Commit a3f5ce76b10a3c0e2a07cd96bd5a29acc42ac532 shifted all method ordinals by 32 bits, e.g. from 0x42bcb6d9 to 0x42bcb6d900000000. And since they are passed in int32, they all effectively become 0 so now syzkaller does not know about any method ordinals.

Now we statically check for such bugs and I had to temporary change field to int64: https://github.com/google/syzkaller/pull/1641#discussion_r393939305

Need to fix fidlgen to generate right consts again.

[dvyukov]

@dpursell FYI

[mvanotti]

I'm temporarily disabling fidl fuzzing in our syz-ci instance until this issue is solved.

[mdempsky]

@dvyukov We're trying to write a test for this, but we're having difficulties writing this in syz-runtest format. The current test we've drafted based on syz-manager-generated tests is:

zx_channel_create$fuchsia_net_NameLookup(0x0, &AUTO=<r0=>0x0, &AUTO=<r1=>0x0)
fdio_service_connect$fuchsia_net_NameLookup(&AUTO='/svc/fuchsia.net.NameLookup\x00', r1)

r2 = zx_deadline_after(0x2)
zx_channel_call$fuchsia_net_NameLookupLookupHostname(r0, 0x0, r2, &AUTO={&AUTO={{}, @ipv6InLine={0x1, {"f0f44bb280c802c\
77303c6425e8478b5"}}, @ipv6OutOfLine}, &AUTO, &AUTO, &AUTO, 0x2c, 0x0, 0x10000}, &AUTO, &AUTO)

zx_handle_close(r0)
zx_handle_close(r1)

syz-runtest gives us this error:

2020/06/02 12:20:07 failed to deserialize fidl_net: missing struct fidl_message_header[1682143474989662208] fields 0/5
line #5:82: zx_channel_call$fuchsia_net_NameLookupLookupHostname(r0, 0x0, r2, &AUTO={&AUTO={{}, @ipv6InLine={0x1, {"f0f44bb280c802c77303c6425e8478b5"}}, @ipv6OutOfLine}, &AUTO, &AUTO, &AUTO, 0x2c, 0x0, 0x10000}, &AUTO, &AUTO)

We're trying to use the syzkaller definitions at https://github.com/google/syzkaller/blob/master/sys/fuchsia/fuchsia_net.syz.txt

[dvyukov]

syz-runtest probably gives you a long turn around time, a faster way is to use any other utility that deserializes programs, e.g.:

make mutate
bin/syz-mutate -os=fuchsia sys/fuchsia/test/fidl

[dvyukov]

Here is the program that deserializes:

zx_channel_create$fuchsia_net_NameLookup(0x0, &AUTO=<r0=>0x0, &AUTO=<r1=>0x0)
fdio_service_connect$fuchsia_net_NameLookup(&AUTO='/svc/fuchsia.net.NameLookup\x00', r1)
r2 = zx_deadline_after(0x2)
zx_channel_call$fuchsia_net_NameLookupLookupHostname(r0, 0x0, r2, &AUTO={&AUTO={{AUTO, AUTO, AUTO, AUTO}, @ipv6InLine={0x1, {"f0f44bb280c802c77303c6425e8478b5"}}, @ipv6OutOfLine}, &AUTO, &AUTO, &AUTO, AUTO, AUTO, AUTO, AUTO}, &AUTO, &AUTO)

There were just some missing fields in structs. fidl_message_header did not contains any, that's where you used {}. And fidl_call_args had 1 missing at the end for rd_num_handles:

missing struct fidl_call_args[fuchsia_net_NameLookupLookupHostnameRequest, fuchsia_net_NameLookupLookupHostnameRequestHandles, array[int8, ZX_CHANNEL_MAX_MSG_BYTES], fuchsia_net_NameLookupLookupHostnameResponseHandles] fields 7/8

It's also better to use AUTO for these:

    wr_num_bytes    bytesize[wr_bytes, int32]
    wr_num_handles  bytesize4[wr_handles, int32]
    rd_num_bytes    bytesize[rd_bytes, int32]
    rd_num_handles  bytesize4[rd_handles, int32]

instead of hard-coded numbers you used: 0x2c, 0x0, 0x10000, because that will test what fuzzer think is the size of these (easier to generate). Otherwise the test may pass with your very special crafted values, but fuzzer will never be able to come up with right values on its own.

[mvanotti]

Thanks Dmitry! We are making some good progress here, and found some places in fidl that need to be fixed. However, it seems like fidl method ordinals have changed to 64 bits. We still need to make sure that fidlgen is using the right constants.

Is there a way to check that the values returned by the call meet certain criteria? Like a memcmp, or something like that? It would be nice to know that a fidl with a given set of parameters returns always the same result.

When you said to use AUTO, we still need to make sure the sizes of those fields match the sizes of the buffers, is there a way to say "hey, this value is the size of that buffer that I'm using there?" or "AUTO" just knows what to use?

The way we created the fidl test call was using syz-manager allowing only a subset of system calls, then grabbed a generated program and tested that. However, the generated program had the issues of not serializing correctly :(

We ended up making the fidl call from scratch, following your template.

[dvyukov]

Is there a way to check that the values returned by the call meet certain criteria? Like a memcmp, or something like that? It would be nice to know that a fidl with a given set of parameters returns always the same result.

Return values of syscalls are always checked in runtest. They are expected to be 0 unless stated otherwise. For indirect outputs we use this syz_compare: https://github.com/google/syzkaller/blob/master/executor/common_test.h#L44 https://github.com/google/syzkaller/blob/master/sys/test/test/bf2

When you said to use AUTO, we still need to make sure the sizes of those fields match the sizes of the buffers, is there a way to say "hey, this value is the size of that buffer that I'm using there?" or "AUTO" just knows what to use?

This is what all this descriptions thing is about. bytesize here means "this is the size of that buffer":

    wr_num_bytes    bytesize[wr_bytes, int32]
    wr_num_handles  bytesize4[wr_handles, int32]
    rd_num_bytes    bytesize[rd_bytes, int32]
    rd_num_handles  bytesize4[rd_handles, int32]

Or what do you mean?

The way we created the fidl test call was using syz-manager allowing only a subset of system calls, then grabbed a generated program and tested that. However, the generated program had the issues of not serializing correctly :(

Tests have stricter checking rules intentionally, otherwise they quickly and silently get out of sync with descriptions. The non-strict mode will successfully deserialize almost any program against any descriptions.