google / syzkaller

syzkaller is an unsupervised coverage-guided kernel fuzzer
Apache License 2.0
5.23k stars 1.2k forks source link

pkg/{ast,compiler}: specify always set flags in the `flags` type #4191

Open a-nogikh opened 10 months ago

a-nogikh commented 10 months ago

From the mailing list:

Hello,

I'm trying to describe an extension of the bpf(2) syscall where there is a new union in a struct akin to the following.

    struct {
        union {
            __u32 relative_fd;
            __u32 relative_id;
        };
        __u32 flags;
    };

The union is interpreted as a BPF ID or an FD depending on whether the flag BPF_F_ID is set. That flag is however not the only possible value, so BPF_F_AFTER|BPF_F_ID for instance would be acceptable.

Is there any way I can make the description more specific to cover this, by having two descriptions for the struct with one where BPF_F_ID is always set? Something like:

    bpf_tcx [
        id bpf_tcx_id
        fd bpf_tcx_fd
    ]
    bpf_tcx_id {
        relative_id bpf_id
        flags       flags[bpf_attach_flags, int32, BPF_F_ID] # BPF_F_ID always set.
    }
    bpf_tcx_id {
        relative_fd bpf_fd
        flags       flags[bpf_attach_flags, int32]
    }

Thank you,
Paul

This could fit nicely in the expression evaluation support feature: e.g. flags[bpf_attach_flags | BPF_F_ID, int32]. But it's unlikely to be implemented soon, so extending the flag type definition makes sense.

We still need to determine:

Cc @pchaigno

N.B. There are also string flags. The feature is not applicable to them.

dvyukov commented 10 months ago

For the second struct we also want something like flags[bpf_attach_flags & ~BPF_F_ID, int32], right? Or, BPF_F_ID is just assumed to not be present in bpf_attach_flags?

a-nogikh commented 10 months ago

FTR https://github.com/google/syzkaller/issues/2226 is an approach that could address the problem from the other side (determine the field depending on flags).

pchaigno commented 10 months ago

Do we need to support several always present flags?

I think that would be useful. The id_or_fd union we have for BPF is a good example. Right now it's a single union for all 4 cases, but ideally we should be able to distinguish based on the flags of the parent structures: bpf_attach_arg and bpf_detach_arg. Flag BPF_F_ID tells us id_or_fd is an ID, as in my original example. But in addition, BPF_F_LINK tells us id_or_fd points to a link (vs. a program).

For the second struct we also want something like flags[bpf_attach_flags & ~BPF_F_ID, int32], right? Or, BPF_F_ID is just assumed to not be present in bpf_attach_flags?

Yeah, that's correct. We'd ideally want to be able to exclude flags as well.

FTR #2226 is an approach that could address the problem from the other side (determine the field depending on flags).

That syntax definitely looks more flexible to me, though I'm guessing it's harder to support.