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

possible buffer overrun of runtime.memory_compare() and runtime.memory_compare_zero() #4405

Open
beaumccartney opened this issue Oct 21, 2024 · 2 comments · May be fixed by #4411
Open

possible buffer overrun of runtime.memory_compare() and runtime.memory_compare_zero() #4405

beaumccartney opened this issue Oct 21, 2024 · 2 comments · May be fixed by #4411
Labels

Comments

@beaumccartney
Copy link
Contributor

using either of the procedures on memory i've mapped myself crashes after trying to dereference just after the last mapped byte (i.e. not valid address space).

Odin Report:

Odin:    dev-2024-10:9f609dd74
OS:      macOS Sequoia 15.0.1 (build: 24A348, kernel: 24.0.0)
CPU:     Apple M2
RAM:     16384 MiB
Backend: LLVM 18.1.8

repro:

package repro

main :: proc() {
    a := reserve_and_commit(mem.DEFAULT_PAGE_SIZE) or_else panic("1")
    b := reserve_and_commit(mem.DEFAULT_PAGE_SIZE) or_else panic("2")

    // prove that sufficient space is mapped
    fmt.println("last a:", a[mem.DEFAULT_PAGE_SIZE-1])
    fmt.println("last b:", b[mem.DEFAULT_PAGE_SIZE-1])
    fmt.println("last b address:", &b[mem.DEFAULT_PAGE_SIZE-1])

    // XXX: all of these crash
    runtime.memory_compare_zero(raw_data(b), len(b))
    runtime.memory_compare(raw_data(a), raw_data(b), len(b))
    fmt.println(mem.compare(a, b))
}


// NOTE: copied directly from core:mem/virtual darwin

MAP_ANONYMOUS :: 0x1000 /* allocated from memory, swap space */
_reserve :: proc "contextless" (size: uint) -> (data: []byte, err: Allocator_Error) {
	flags  := posix.Map_Flags{ .PRIVATE } + transmute(posix.Map_Flags)i32(MAP_ANONYMOUS)
	result := posix.mmap(nil, size, {}, flags)
	if result == posix.MAP_FAILED {
		return nil, .Out_Of_Memory
	}

	return ([^]byte)(uintptr(result))[:size], nil
}

_commit :: proc "contextless" (data: rawptr, size: uint) -> Allocator_Error {
	if posix.mprotect(data, size, { .READ, .WRITE }) != .OK {
		return .Out_Of_Memory
	}

	return nil
}

reserve_and_commit :: proc "contextless" (size: uint) -> (data: []byte, err: Allocator_Error) {
	data = _reserve(size) or_return
	_commit(raw_data(data), size) or_return
	return
}


Allocator_Error :: runtime.Allocator_Error

import "core:fmt"
import "core:mem"
import "core:sys/posix"
import "base:runtime"
@beaumccartney
Copy link
Contributor Author

also using memcmp from libc or something doesn't crash

@laytan laytan added the bug label Oct 22, 2024
@tf2spi
Copy link
Contributor

tf2spi commented Oct 22, 2024

In runtime.memory_compare_zero

	fast := n/SU + 1

This way of calculating fast looks to be an off-by-one error when considering this comparison loop.

	curr_block := uintptr(0)
// ...
	for /**/; curr_block < fast; curr_block += 1 {
		va := (^uintptr)(x + curr_block * size_of(uintptr))^
// ...

At the last iteration, curr_block would get 2048 on Darwin ARM64.

I think there are also alignment issues with this as well since x could've been an unaligned pointer.

@tf2spi tf2spi linked a pull request Oct 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants