linux-kernel - Re: [PATCH 2/2] ARM: mm: make text and rodata read-only

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jJq24k4uQbBJmihJ4RScJ40o41oKFtm13hBO_YZOvHpkA@mail.gmail.com>
Date:	Tue, 8 Apr 2014 09:01:36 -0700
From:	Kees Cook <keescook@...omium.org>
To:	"Jon Medhurst (Tixy)" <tixy@...aro.org>
Cc:	Rabin Vincent <rabin@....in>,
	Russell King <linux@....linux.org.uk>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	LKML <linux-kernel@...r.kernel.org>,
	Laura Abbott <lauraa@...eaurora.org>,
	Alexander Holler <holler@...oftware.de>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 2/2] ARM: mm: make text and rodata read-only

On Tue, Apr 8, 2014 at 5:41 AM, Jon Medhurst (Tixy) <tixy@...aro.org> wrote:
> On Fri, 2014-04-04 at 17:07 -0700, Kees Cook wrote:
>> On Fri, Apr 4, 2014 at 12:58 PM, Rabin Vincent <rabin@....in> wrote:
> [...]
>> > You need a TLB flush.  I had a flush_tlb_all() in my example patch,
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244335.html,
>> > but the following is probably nicer (on top of this patch):
>> >
>> > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
>> > index 9bea524..a92c45a 100644
>> > --- a/arch/arm/mm/init.c
>> > +++ b/arch/arm/mm/init.c
>> > @@ -741,6 +741,8 @@ static inline bool arch_has_strict_perms(void)
>> >                      addr += SECTION_SIZE)                              \
>> >                         section_update(addr, perms[i].mask,             \
>> >                                        perms[i].field);                 \
>> > +                                                                       \
>> > +               flush_tlb_kernel_range(perms[i].start, perms[i].end);   \
>> >         }                                                               \
>> >  }
>> >
>>
>> When I do this, I hang the system, and get a WARN due to the tlb call
>> attempting to flush on all CPUs, I think:
>>
>> [   34.246034] WARNING: at
>> /mnt/host/source/src/third_party/kernel-next/kernel/smp.c:466
>> smp_call_function_many+0xac/0x26c()
>> ...
>> [   34.246617] Backtrace:
>> [   34.246697] [<c010d3b8>] (unwind_backtrace+0x0/0x118) from
>> [<c060b9d8>] (dump_stack+0x28/0x30)
>> [   34.246765] [<c060b9d8>] (dump_stack+0x28/0x30) from [<c0123044>]
>> (warn_slowpath_null+0x44/0x5c)
>> [   34.246824] [<c0123044>] (warn_slowpath_null+0x44/0x5c) from
>> [<c017426c>] (smp_call_function_many+0xac/0x26c)
>> [   34.246881] [<c017426c>] (smp_call_function_many+0xac/0x26c) from
>> [<c0174468>] (smp_call_function+0x3c/0x48)
>> [   34.246937] [<c0174468>] (smp_call_function+0x3c/0x48) from
>> [<c010c0fc>] (broadcast_tlb_a15_erratum+0x40/0x4c)
>> [   34.246994] [<c010c0fc>] (broadcast_tlb_a15_erratum+0x40/0x4c) from
>> [<c010c590>] (flush_tlb_kernel_range+0x74/0xa0)
>> [   34.247046] [<c010c590>] (flush_tlb_kernel_range+0x74/0xa0) from
>> [<c011403c>] (set_kernel_text_rw+0xd8/0xec)
>> [   34.247099] [<c011403c>] (set_kernel_text_rw+0xd8/0xec) from
>> [<c010c878>] (__ftrace_modify_code+0x14/0x28)
>> [   34.247156] [<c010c878>] (__ftrace_modify_code+0x14/0x28) from
>> [<c0184318>] (stop_machine_cpu_stop+0xc0/0x114)
>> [   34.247212] [<c0184318>] (stop_machine_cpu_stop+0xc0/0x114) from
>> [<c01841cc>] (cpu_stopper_thread+0xd8/0x164)
>> [   34.247266] [<c01841cc>] (cpu_stopper_thread+0xd8/0x164) from
>> [<c0145c14>] (kthread+0xc8/0xd8)
>> [   34.247323] [<c0145c14>] (kthread+0xc8/0xd8) from [<c0106118>]
>> (ret_from_fork+0x14/0x20)
>>
>> Using local_flush_tlb_kernel_range() fixed it though.
>
> What about if another CPU had a TLB entry with the old permissions in?
> Or do you consider that the likelihood and consequences of that aren't
> significant?

The purpose of the function is to temporarily make text writable, do
the write, and then restore read-only. Since only the writer needs to
care about TLB state, this works fine. It's actually nice that only
the current CPU can make text writes.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/