[RFC PATCH] arm64: fault: Don't leak data in ESR context for user fault on kernel VA

From: Dave.Martin@arm.com (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
Date: Wed, 18 Apr 2018 11:33:13 +0100	[thread overview]
Message-ID: <20180418103311.GH16308@e103592.cambridge.arm.com> (raw)
In-Reply-To: <CAFEAcA_MKzoNV3rAs=wkfxjSXL8twOWcMxcaXct-o=WFmEVMnA@mail.gmail.com>

On Tue, Apr 17, 2018 at 06:34:11PM +0100, Peter Maydell wrote:
> On 13 April 2018 at 15:21, Dave Martin <Dave.Martin@arm.com> wrote:
> > There are certain fault types that should never be exposed to
> > userspace, such as TLB conflict aborts, access flag faults, etc.
> > See the changes upstream between v4.16 and torvalds/master (which I had
> > temporarily forgotten about...)
> >
> > Now, __do_user_fault() is called for these cases but the signal has
> > already been mapped to SIGKILL by this point via the fault_info[]
> > table.  It doesn't matter too much what we do in such cases because
> > the user task will be killed on signal delivery and so doesn't get a
> > chance to inspect the ESR contents anyway.  It might be worth expanding
> > the WARN() to catch mismaintenance of the fault_info[] table -- but that
> > may be overkill.  Maybe adding some comments explaining the dependency
> > is sufficient.
> 
> I had a look at the code in current master, and from my reading
> of it it does not call __do_user_fault() for faults like TLB
> conflict aborts, etc. Those all have fault_info[] table entries
> that specify do_bad() as the function pointer, which just returns 1.
> It's only do_page_fault(), do_translation_fault() and do_alignment()
> fault that can get into __do_user_fault().
> 
> So I don't think there's any change that needs to be made for this
> point. Am I misunderstanding what you have in mind? If so it would
> probably be simplest if you explained the change you'd like to see.

Don't worry about this; I think you're right.  I was confusing do_bad()
with do_bad_area() in my head somewhere along the line...

> 
> (The changes in master since v4.16 do require a rebase anyway, mostly
> for textual reasons, so I'll base v2 on master unless you have a
> different branch you'd rather I based it on.)

-rc1 or master is probably fine.  Most of the dust from the merge window
ought to have settled now.

> I've made the various other changes you suggest.

Cheers
---Dave