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

Unaligned 64bit load in ubpf_vm.c @ line 444 #149

Closed
Alan-Jowett opened this issue Oct 10, 2022 · 3 comments · Fixed by #119 or #154
Closed

Unaligned 64bit load in ubpf_vm.c @ line 444 #149

Alan-Jowett opened this issue Oct 10, 2022 · 3 comments · Fixed by #119 or #154

Comments

@Alan-Jowett
Copy link
Collaborator

https://github.com/iovisor/ubpf/actions/runs/3223008829/jobs/5272650798#step:14:184

/Users/runner/work/ubpf/ubpf/vm/ubpf_vm.c:444:29: runtime error: load of misaligned address 0x602000000132 for type 'uint64_t' (aka 'unsigned long long'), which requires 8 byte alignment
0x602000000132: note: pointer points here
00 00 aa bb 11 22 33 44 55 66 77 88 cc dd be be be be 02 11 00 00 10 00 00 00 01 00 00 0e 00 00

        case EBPF_OP_LDXDW:
            BOUNDS_CHECK_LOAD(8);
            reg[inst.dst] = *(uint64_t *)(uintptr_t)(reg[inst.src] + inst.offset);
            break;

@matt-gretton-dann do you happen to know what processor type the CICD is running on? I haven't seen warnings like this since Itanium. Not a Mac expert myself.

@matt-gretton-dann
Copy link
Collaborator

@Alan-Jowett : Looking at the docs these are x86-64 machines (https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources).

However, the macOS ABI seems to assume natural alignment of types, and so I wonder if ASan catches this as technically undefined behaviour.

@Alan-Jowett
Copy link
Collaborator Author

OK, that makes sense. I can update this with an alignment check and a memcpy for unaligned loads. I think:

#ifdef __APPLE__

Is the correct MacOS check.

@matt-gretton-dann
Copy link
Collaborator

I've also integrated a fix to this into #154.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants