arm64: __clear_user: handle exceptions on strb
PATCH 1/1 arm64: __clear_user: handle exceptions on strb
ARM64 currently doesn't fix up faults on the single-byte (strb) case of __clear_user... which means that we can cause a nasty kernel panic as an ordinary user with any multiple PAGE_SIZE+1 read from /dev/zero. i.e.: dd if=/dev/zero of=foo ibs=1 count=1 (or ibs=65537, etc.)
This is a pretty obscure bug in the general case since we'll only __do_kernel_fault (since there's no extable entry for pc) if the mmap_sem is contended. However, with CONFIG_DEBUG_VM enabled, we'll always fault.
if (!down_read_trylock(&mm->mmap_sem)) {
if (!user_mode(regs) && !search_exception_tables(regs->pc))
goto no_context;
retry:
down_read(&mm->mmap_sem);
} else {
/*
* The above down_read_trylock() might have succeeded in
* which
* case, we'll have missed the might_sleep() from
* down_read().
*/
might_sleep();
#ifdef CONFIG_DEBUG_VM
if (!user_mode(regs) && !search_exception_tables(regs->pc))
goto no_context;#endif
}
Fix that by adding an extable entry for the strb instruction, since it touches user memory, similar to the other stores in __clear_user.
arch/arm64/lib/clear_user.S +1 -1
@@ -46,7 +46,7 @@ USER(9f, strh wzr, [x0], #2 ) sub x1, x1, #2 4: adds x1, x1, #1 b.mi 5f - strb wzr, [x0] +USER(9f, strb wzr, [x0] ) 5: mov x0, #0 ret ENDPROC(__clear_user)
Catalin Marinas
maintainerQuoted 4 lines · Kyle McMartin
ARM64 currently doesn't fix up faults on the single-byte (strb) case of
__clear_user... which means that we can cause a nasty kernel panic as an ordinary user with any multiple PAGE_SIZE+1 read from/dev/zero. i.e.: dd if=/dev/zero of=foo ibs=1 count=1 (or ibs=65537, etc.)
Thanks for this, it's been like this for a while. Applied.
Signature · 1 line
Catalin
Quoted 8 lines · Catalin Marinas
Quoted 4 lines · Kyle McMartin
ARM64 currently doesn't fix up faults on the single-byte (strb) case of
__clear_user... which means that we can cause a nasty kernel panic as an ordinary user with any multiple PAGE_SIZE+1 read from/dev/zero. i.e.: dd if=/dev/zero of=foo ibs=1 count=1 (or ibs=65537, etc.)Thanks for this, it's been like this for a while. Applied.
Thanks Catalin, if it's not too late, I forgot to add a
for the initial report of the panic.
regards, Kyle
Catalin Marinas
maintainerQuoted 7 lines · Kyle McMartin
Quoted 6 lines · Catalin Marinas
Quoted 4 lines · Kyle McMartin
ARM64 currently doesn't fix up faults on the single-byte (strb) case of
__clear_user... which means that we can cause a nasty kernel panic as an ordinary user with any multiple PAGE_SIZE+1 read from/dev/zero. i.e.: dd if=/dev/zero of=foo ibs=1 count=1 (or ibs=65537, etc.)
Quoted 6 lines
Quoted 2 lines
Thanks for this, it's been like this for a while. Applied.
Thanks Catalin, if it's not too late, I forgot to add a
Reported-by Milo? Prchl?k <[email protected]>for the initial report of the panic.
I'll add this, I haven't pushed it out yet.
Signature · 1 line
Catalin