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

Add exec option to mmap #53463

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

fatteneder
Copy link
Member

@fatteneder fatteneder commented Feb 25, 2024

mmap(exec=true) should allow one to generate a buffer that is writeable and (with enough precautions taken) can be executed.

We only implement exec=true for non-file-backed mmap, in which we request a new file handle from the OS, as otherwise we would have to adjust permissions on whatever file handle we received, which might not be possible in all cases.


MWE. Don't try this at home

# mwe_exec.jl
using Mmap

buf = mmap(Vector{UInt8}, 6, shared=false, exec=true)
function myidentity(x::Int32)
    ux = unsigned(x)
    buf[1] = 0xb8
    buf[2:5] .= reinterpret(UInt8, [ux])
    buf[6] = 0xc3
    return ccall(pointer(buf), Cint, ())
end

for x in rand(Int32, 5)
    @show x == myidentity(x)
end
julia> versioninfo()
Julia Version 1.12.0-DEV.5
Commit 1c13b8b90f (2024-02-17 22:20 UTC)
Platform Info:
  OS: Linux (x86_64-unknown-linux-gnu)
  CPU: 12 × Intel(R) Core(TM) i7-5820K CPU @ 3.30GHz
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, haswell)
Threads: 1 default, 0 interactive, 1 GC (on 12 virtual cores)

julia> include("mwe_exec.jl")
x == myidentity(x) = true
x == myidentity(x) = true
x == myidentity(x) = true
x == myidentity(x) = true
x == myidentity(x) = true
  • Anyone knows why this segfaults when either buf is marked const or buf initialization is moved into myidentity?

@fatteneder fatteneder added the stdlib Julia's standard library label Feb 25, 2024
@vtjnash
Copy link
Member

vtjnash commented Feb 25, 2024

Most CPUs define that it is UB to execute code immediately after writing to the memory. They usually require some sort of flush, fence, and icache invalidation calls before that is legal

stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
@fatteneder
Copy link
Member Author

fatteneder commented Feb 26, 2024

TODO:

@fatteneder
Copy link
Member Author

fatteneder commented Feb 28, 2024

Regarding windows failures:
I did some more reading about windows' CreatFileMapping and friends.
Assuming GENERIC_EXECUTE is really the problem, I would have to mess with the ACLs of the file handle that underlies the IO that we pass to mmap.
Not sure if that's even feasible to implement, but I rather don't.

Instead, I was thinking about limiting the exec option to only work with the Anonymous() dispatch, e.g. none file backed mmap.
This way I could just request a file handle using VirtualAlloc and the right access rights and all should be good (trademark).
No need for VirtualAlloc, because the existing setup just works with non-file-backed mmap.

Need to dust off my windows VM to see if I can implement that locally.

fatteneder and others added 3 commits March 2, 2024 13:15
stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
@semarie
Copy link
Contributor

semarie commented Mar 3, 2024

I would like to note that WX permissions could be restricted

@fatteneder
Copy link
Member Author

I would like to note that WX permissions could be restricted

I updated the PR description and I think this is no longer an issue, because we only implement exec for non-file-backed mmap, e.g. we query the handle with the desired rights ourselves. (but atm there is still an edge case I need to patch)

stdlib/Mmap/src/Mmap.jl Show resolved Hide resolved
stdlib/Mmap/src/Mmap.jl Show resolved Hide resolved
stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
@fatteneder fatteneder requested a review from vtjnash March 5, 2024 00:17
stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
stdlib/Mmap/src/Mmap.jl Outdated Show resolved Hide resolved
fatteneder and others added 4 commits March 10, 2024 21:23
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants