[PATCH v5 3/8] arm: fixmap: implement __set_fixmap()

[Kees Cook]

On Mon, Sep 8, 2014 at 2:55 PM, Rabin Vincent <rabin at rab.in> wrote:
> On Mon, Sep 08, 2014 at 12:16:34PM -0700, Kees Cook wrote:
>> On Thu, Sep 04, 2014 at 06:27:48PM +0100, Will Deacon wrote:
>> > On Thu, Sep 04, 2014 at 06:23:42PM +0100, Kees Cook wrote:
>> > > Ah! If this is the case, perhaps we can get away with
>> > > local_flush_tlb_kernel_range() then?
>> >
>> > That's a bit tricky, since you need to ensure that preemption is disabled
>> > until the mapping is put back like it was.
>>
>> Okay, under both real hardware with the errata, and under QEMU, things seem
>> to work with this change to the series. What do you think?
>
> Preemption is already disabled until the mapping is put back in this
> patch.c code because interrupts are disabled from before the time
> set_fixmap() is called until after clear_fixmap() is called.

Should I drop the preempt_disable/enable(), and just add a comment to
set_fixmap()?

> I'd guess that Will meant other (future) callers of set_fixmap() would
> have to ensure similar behaviour with set_fixmap() / clear_fixmap().
>
> Unless I'm missing something set/clear_fixmap() seem to be quite arch
> specific and only really used on x86, so we could ensure that future
> users on ARM perform the correct tlb flush:  the first user on ARM with
> a non-atomic context (or you) could implement a set_fixmap() which does
> the global flush and have this patch.c (and any other atomic context
> callers) call __set_fixmap() directly.
>
> The change to local_flush_tlb_kernel_range() in __set_fixmap() would of
> course be needed in that case, and IIRC that was what my original patch
> had (via set_top_pte()).

Ah, so it was, yes! Will, which version of this logic would you prefer?

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security