linux-kernel - Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early

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]

Date:	Sun, 3 Apr 2016 06:22:10 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Borislav Petkov <bp@...en8.de>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	KVM list <kvm@...r.kernel.org>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	xen-devel <Xen-devel@...ts.xen.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early

On Sun, Apr 3, 2016 at 1:07 AM, Borislav Petkov <bp@...en8.de> wrote:
> On Sat, Apr 02, 2016 at 10:52:48PM +0200, Borislav Petkov wrote:
>> On Sat, Apr 02, 2016 at 01:16:07PM -0700, Andy Lutomirski wrote:
>> > I have no idea why it was explicitly unsupported, but I'm guessing it
>> > was just to avoid duplicating the code.  Early "ext" uaccess failures
>> > are certainly not going to work, but I don't think this is a problem
>> > -- there's no userspace before trap_init runs, so how exactly is an
>> > "ext" uaccess going to happen in the first place?
>> >
>> > In any event, if it did happen in older kernels, it would have
>> > immediately panicked due to that code.  At least with my code it just
>> > might manage to EFAULT correctly.
>>
>> Yeah, I was wondering what that early thing meant.
>>
>> Linus or tip guys probably remember what this whole deal with early
>> uaccess was about. I'll try to do some git archeology tomorrow.
>
> Yep, just as I suspected:
>
> 6a1ea279c210 ("x86, extable: Add early_fixup_exception()")
>
> Apparently, thread_info might not have been setup yet. I'm guessing the
> intention behind this no-uaccess-fixup-early is to not even attempt any
> fixup due to stuff *probably* not initialized yet and so the safer thing
> would be to panic instead.
>
> I'm wondering whether making it try to EFAULT correctly is the right
> thing to do... We're certainly more conservative if we panic and not
> allow some silently failed attempt at recovery which looks successful,
> to continue.
>
> hpa, thoughts?

I don't think this matters much.  There aren't many users of this
mechanism in the tree:

./arch/x86/kernel/signal.c:    get_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal_compat.c:    put_user_try {
./arch/x86/kernel/signal_compat.c:    get_user_try {
./arch/x86/kernel/vm86_32.c:    put_user_try {
./arch/x86/kernel/vm86_32.c:    get_user_try {
./arch/x86/ia32/ia32_signal.c:    get_user_try {
./arch/x86/ia32/ia32_signal.c:    put_user_try {
./arch/x86/ia32/ia32_signal.c:    put_user_try {
./arch/x86/ia32/ia32_signal.c:    put_user_try {
./arch/x86/include/asm/uaccess.h: * {get|put}_user_try and catch
./arch/x86/include/asm/uaccess.h: * get_user_try {
./arch/x86/include/asm/uaccess.h:#define get_user_try        uaccess_try
./arch/x86/include/asm/uaccess.h:#define put_user_try        uaccess_try

I don't see how we could get to that code in the first place without
current_thread_info() working.

If we can ever convince gcc to do jump labels properly for uaccess, it
would probably be better to just delete all that code.

--Andy