Skip to content

Commit 13d775f

Browse files
committed
Make ReadOnlyError on PROT_NONE consistent
This replaces #36625. Essentially the question is what should reading from PROT_NONE memory do. At the moment we throw ReadOnlyMemoryError on Linux and pass through the segfault on Windows/Mac. It's not super clear what the right answer is, though it would be nice to be consistent. After some discussion, it seemed that not throwing ReadOnlyMemoryError on Linux was probably the better consistency direction for two reasons: 1. Getting a ReadOnlyMemoryError is confusing if the failing operation is a read. We could of course rename the error, but that would be a bigger breaking change. 2. The purpose of this exception is to protect people who accidentally try to write to mmap'ed memory that they don't have write permissions to. It was never intended to become a general memory access error exception, so it seems prudent to restrict it as much as possible. To make this happen, there is some platform specific code here to read the memory error register out of the signal context. Decoding this register is shared code between the operating systems, but the register itself is in an OS-specifc location in the signal frame. I have implemented the retrieval code for all our CI'ed platforms. Other users will get an appropriate `#warning` (similar to other code in this file) and can fill in the necessary code at their leisure. A new test in test/misc.jl will indicate whether or not the check is correct.
1 parent 8e949d6 commit 13d775f

File tree

3 files changed

+91
-10
lines changed

3 files changed

+91
-10
lines changed

src/signals-mach.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,6 @@ typedef x86_exception_state64_t host_exception_state_t;
146146
#define HOST_EXCEPTION_STATE x86_EXCEPTION_STATE64
147147
#define HOST_EXCEPTION_STATE_COUNT x86_EXCEPTION_STATE64_COUNT
148148

149-
enum x86_trap_flags {
150-
USER_MODE = 0x4,
151-
WRITE_FAULT = 0x2,
152-
PAGE_PRESENT = 0x1
153-
};
154-
155149
#elif defined(_CPU_AARCH64_)
156150
typedef arm_thread_state64_t host_thread_state_t;
157151
typedef arm_exception_state64_t host_exception_state_t;
@@ -167,11 +161,11 @@ static void jl_call_in_state(jl_ptls_t ptls2, host_thread_state_t *state,
167161
uint64_t rsp = (uint64_t)ptls2->signal_stack + sig_stack_size;
168162
assert(rsp % 16 == 0);
169163

164+
#ifdef _CPU_X86_64_
170165
// push (null) $RIP onto the stack
171166
rsp -= sizeof(void*);
172167
*(void**)rsp = NULL;
173168

174-
#ifdef _CPU_X86_64_
175169
state->__rsp = rsp; // set stack pointer
176170
state->__rip = (uint64_t)fptr; // "call" the function
177171
#else
@@ -180,6 +174,21 @@ static void jl_call_in_state(jl_ptls_t ptls2, host_thread_state_t *state,
180174
#endif
181175
}
182176

177+
#ifdef _CPU_X86_64_
178+
int is_write_fault(host_exception_state_t exc_state) {
179+
return exc_reg_is_write_fault(exc_state.__err);
180+
}
181+
#elif defined(_CPU_AARCH64_)
182+
int is_write_fault(host_exception_state_t exc_state) {
183+
return exc_reg_is_write_fault(exc_state.__esr);
184+
}
185+
#else
186+
#warning Implement this query for consistent PROT_NONE handling
187+
int is_write_fault(host_exception_state_t exc_state) {
188+
return 0;
189+
}
190+
#endif
191+
183192
static void jl_throw_in_thread(int tid, mach_port_t thread, jl_value_t *exception)
184193
{
185194
unsigned int count = THREAD_STATE_COUNT;
@@ -279,8 +288,8 @@ kern_return_t catch_exception_raise(mach_port_t exception_port,
279288
}
280289
#endif
281290
else {
282-
if (!(exc_state.__err & WRITE_FAULT))
283-
return KERN_INVALID_ARGUMENT; // rethrow the SEGV since it wasn't an error with writing to read-only memory
291+
if (!is_write_fault(exc_state))
292+
return KERN_INVALID_ARGUMENT;
284293
excpt = jl_readonlymemory_exception;
285294
}
286295
jl_throw_in_thread(tid, thread, excpt);

src/signals-unix.c

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,10 +225,70 @@ static void sigdie_handler(int sig, siginfo_t *info, void *context)
225225
// fall-through return to re-execute faulting statement (but without the error handler)
226226
}
227227

228+
#if defined(_CPU_X86_64_) || defined(_CPU_X86_)
229+
enum x86_trap_flags {
230+
USER_MODE = 0x4,
231+
WRITE_FAULT = 0x2,
232+
PAGE_PRESENT = 0x1
233+
};
234+
235+
int exc_reg_is_write_fault(uintptr_t err) {
236+
return err & WRITE_FAULT;
237+
}
238+
#elif defined(_CPU_AARCH64_)
239+
enum aarch64_esr_layout {
240+
EC_MASK = ((uint32_t)0b111111) << 25,
241+
EC_DATA_ABORT = ((uint32_t)0b100100) << 25,
242+
ISR_DA_WnR = ((uint32_t)1) << 6
243+
};
244+
245+
int exc_reg_is_write_fault(uintptr_t esr) {
246+
return (esr & EC_MASK) == EC_DATA_ABORT && (esr & ISR_DA_WnR);
247+
}
248+
#endif
249+
228250
#if defined(HAVE_MACH)
229251
#include "signals-mach.c"
230252
#else
231253

254+
255+
#if defined(_OS_LINUX_) && (defined(_CPU_X86_64_) || defined(_CPU_X86_))
256+
int is_write_fault(void *context) {
257+
ucontext_t *ctx = (ucontext_t*)context;
258+
return exc_reg_is_write_fault(ctx->uc_mcontext.gregs[REG_ERR]);
259+
}
260+
#elif defined(_OS_LINUX_) && defined(_CPU_AARCH64_)
261+
struct linux_aarch64_ctx_header {
262+
uint32_t magic;
263+
uint32_t size;
264+
};
265+
const uint32_t linux_esr_magic = 0x45535201;
266+
267+
int is_write_fault(void *context) {
268+
ucontext_t *ctx = (ucontext_t*)context;
269+
struct linux_aarch64_ctx_header *extra =
270+
(struct linux_aarch64_ctx_header *)ctx->uc_mcontext.__reserved;
271+
while (extra->magic != 0) {
272+
if (extra->magic == linux_esr_magic) {
273+
return exc_reg_is_write_fault(*(uint64_t*)&extra[1]);
274+
}
275+
extra = (struct linux_aarch64_ctx_header *)
276+
(((uint8_t*)&extra[1]) + extra->size);
277+
}
278+
return 0;
279+
}
280+
#elif defined(_OS_FREEBSD_) && (defined(_CPU_X86_64_) || defined(_CPU_X86_))
281+
int is_write_fault(void *context) {
282+
ucontext_t *ctx = (ucontext_t*)context;
283+
return exc_reg_is_write_fault(ctx->uc_mcontext.mc_err);
284+
}
285+
#else
286+
#warning Implement this query for consistent PROT_NONE handling
287+
int is_write_fault(void *context) {
288+
return 0;
289+
}
290+
#endif
291+
232292
static int is_addr_on_sigstack(jl_ptls_t ptls, void *ptr)
233293
{
234294
// One guard page for signal_stack.
@@ -273,7 +333,7 @@ static void segv_handler(int sig, siginfo_t *info, void *context)
273333
jl_safe_printf("ERROR: Signal stack overflow, exit\n");
274334
_exit(sig + 128);
275335
}
276-
else if (sig == SIGSEGV && info->si_code == SEGV_ACCERR) { // writing to read-only memory (e.g., mmap)
336+
else if (sig == SIGSEGV && info->si_code == SEGV_ACCERR && is_write_fault(context)) { // writing to read-only memory (e.g., mmap)
277337
jl_throw_in_ctx(ptls, jl_readonlymemory_exception, sig, context);
278338
}
279339
else {

test/misc.jl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,3 +952,15 @@ end
952952
@testset "issue #28188" begin
953953
@test `$(@__FILE__)` == let file = @__FILE__; `$file` end
954954
end
955+
956+
# Test that read fault on a prot-none region does not incorrectly give
957+
# ReadOnlyMemoryEror, but rather crashes the program
958+
const MAP_ANONYMOUS_PRIVATE = Sys.isbsd() ? 0x1002 : 0x22
959+
let script = :(let ptr = Ptr{Cint}(ccall(:jl_mmap, Ptr{Cvoid},
960+
(Ptr{Cvoid}, Csize_t, Cint, Cint, Cint, Int),
961+
C_NULL, 16*1024, 0, $MAP_ANONYMOUS_PRIVATE, -1, 0)); try
962+
unsafe_load(ptr)
963+
catch e; println(e) end; end)
964+
@test !success(`$(Base.julia_cmd()) -e $script`)
965+
end
966+

0 commit comments

Comments
 (0)