2014-11-12 21:08:23

by Kyle McMartin

Subject: [PATCH] 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.

Signed-off-by: Kyle McMartin <[email protected]>
Cc: [email protected]

--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -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)

2014-11-13 15:06:36

by Catalin Marinas

Subject: Re: [PATCH] arm64: __clear_user: handle exceptions on strb

On Wed, Nov 12, 2014 at 09:07:44PM +0000, Kyle McMartin wrote:
> 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.

--
Catalin

2014-11-13 15:14:40

by Kyle McMartin

Subject: Re: [PATCH] arm64: __clear_user: handle exceptions on strb

On Thu, Nov 13, 2014 at 03:06:25PM +0000, Catalin Marinas wrote:
> On Wed, Nov 12, 2014 at 09:07:44PM +0000, Kyle McMartin wrote:
> > 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
Reported-by: Miloš Prchlík <[email protected]>
for the initial report of the panic.

regards, Kyle

2014-11-13 15:21:31

by Catalin Marinas

Subject: Re: [PATCH] arm64: __clear_user: handle exceptions on strb

On Thu, Nov 13, 2014 at 03:14:08PM +0000, Kyle McMartin wrote:
> On Thu, Nov 13, 2014 at 03:06:25PM +0000, Catalin Marinas wrote:
> > On Wed, Nov 12, 2014 at 09:07:44PM +0000, Kyle McMartin wrote:
> > > 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
> 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.

--
Catalin