load program: permission denied: !read_ok

[cuizhipeng326]

Describe the bug
I want to use eBPF to filter ETH IP packet by IP protocol in the way of xdp in the examples, so I coded like below:

/*
Attempt to parse the IPv4 source address, protocol, and source port(if protocol is TCP/UDP) from the packet.
Returns 0 if there is no IPv4 header field; otherwise returns non-zero.
*/
static __always_inline int parse_ip_src_triple(struct xdp_md *ctx, __u32 *ip_src_addr, __be16 *port, __u8 *protocol) {
        void *data_end = (void *)(long)ctx->data_end;
        void *data     = (void *)(long)ctx->data;

        // First, parse the ethernet header.
        struct ethhdr *eth = data;
        if ((void *)(eth + 1) > data_end) {
                return 0;
        }

        if (eth->h_proto != bpf_htons(ETH_P_IP)) {
                // The protocol is not IPv4, so we can't parse an IPv4 source address.
                return 0;
        }

//      printf("eth->h_proto:%d, ETH_P_IP:%d\n", eth->h_proto, bpf_htons(ETH_P_IP));
        // Then parse the IP header.
        struct iphdr *ip = (void *)(eth + 1);
        if ((void *)(ip + 1) > data_end) {
                return 0;
        }

        // Return the source IP address in network byte order.
        *ip_src_addr = (__u32)(ip->saddr);

        // Parse protocol
        __u32 p = (__u32)(ip->protocol);
        *protocol = (__u8)(ip->protocol);

        /*switch (ip->protocol) {
                case IPPROTO_TCP:
                        {
                                // Parse tcp port
                                struct tcphdr *tcp = (void *)(ip + 1);
                                if ((void *)(tcp + 1) > data_end) {
                                        return 0;
                                }

                                *port = tcp->source;

                                return 1;
                        }
                case IPPROTO_UDP:
                        {
                                // Parse udp port
                                struct udphdr *udp = (void *)(ip + 1);
                                if ((void *)(udp + 1) > data_end) {
                                        return 0;
                                }

                                *port = udp->source;

                                return 1;
                        }
                case IPPROTO_ICMP: // error occurs when execute
                        {
                                return 1;
                        }
                default:
                        return 0;

        }*/


        //if (ip->protocol == IPPROTO_ICMP) { // outrageous! error occurs when execute
        //if (ip->protocol == htons(IPPROTO_ICMP)) { // no error occurs when execute, but it can not assert if it’s IPPROTO_ICMP or not correctly
           //return 1;
        //}

        if (ip->protocol == IPPROTO_TCP) {
            // Parse tcp port
            struct tcphdr *tcp = (void *)(ip + 1);
            if ((void *)(tcp + 1) > data_end) {
                return 0;
            }

            *port = tcp->source;

            return 1;
        }

        if (ip->protocol == IPPROTO_UDP) {
            // Parse udp port
            struct udphdr *udp = (void *)(ip + 1);
            if ((void *)(udp + 1) > data_end) {
                return 0;
            }

            *port = udp->source;

            return 1;
        }

        return 0;
}

the final binary file executed failed with the code to judge if the protocol is IPPROTO_ICMP, throw an error below:

loading objects: field XdpProgPacketV4: program xdp_prog_packet_v4: load program: permission denied: 30: (73) *(u8 *)(r10 -10) = r0: R0 !read_ok (49 line(s) omitted)

and it executed successfully without the code to judge IPPROTO_ICMP.

To Reproduce
Parse IP packet's IP protocol and judge if it is IPPROTO_ICMP.

go version: go version go1.20 linux/amd64
uname -a: Linux wudun 5.16.0-051600-generic #202201092355 SMP PREEMPT Mon Jan 10 00:21:11 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
cat /etc/issue: Ubuntu 20.04.6 LTS \n \l

Expected behavior
The binary file within the process of judge if the IP protocol is ICMP executes successfully.

[lmb]

!read_ok indicates that you are trying to read from a pointer which has not been checked against NULL and therefore isn't proven to be safe. Can you amend your issue to include the full C code?

[cuizhipeng326]

Sorry for the time difference. I just got your reply. I have added that full C function code, please help to check.

[lmb]

I misread the error you gave, sorry. This is the important part:

*(u8 *)(r10 -10) = r0: R0 !read_ok

This tells you:

• A write of a single u8
• Into the stack at offset - 10 (r10 is the stack pointer)
• Fails because r0 has not been initialized
• r0 is the register used for return values

I'm guessing that your problem isn't actually in parse_ip_src_triple but in the caller. Try and check any place where you're writing into a u8 local variable and see whether there is a way that write might be uninitialized. The fact that changing the code to use htons(IPPROTO_ICMP) might just be clang being very smart and eliminating your code.

For the future, please post questions like this on StackOverflow.

[cuizhipeng326]

First of all, thanks a lot for your help.
It works very well after I initialized the three arguments those are passed in parse_ip_src_triple in the caller. It reminds me that there is no zero-initialization in C compiler by default which like the Golang compiler does, again. Then I took a test: initializing these three arguments one by one to find out which uninitialized argument will cause that problem, it showed the one must to be initialized argument is port. I don't understand why this happened. But I will try to initialize every variable used in C, cause it is important.
And I took an another test: all the above test are based on the clang cflags -O3, apparently -O3 does not work for the initialization of local variable. Then I changed the clang cflags to -ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang, and it works with uninitialized local variable well, too.

[ti-mo]

@cuizhipeng326 Note that for the time being, you'll need to use -O2 for compiling bpf programs. IIRC, LLVM's higher optimization levels generate invalid instructions for helper calls. It's mentioned somewhere in https://lwn.net/Articles/831402, not sure if this is officially documented.

[cuizhipeng326]

Got it, thanks!