linux-kernel - question about save_xstate_sig()

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-next>] [day] [month] [year] [list]

Date:	Fri, 23 Jan 2015 14:34:29 -0500
From:	Rik van Riel <riel@...hat.com>
To:	Suresh Siddha <suresh.b.siddha@...el.com>
CC:	Andy Lutomirski <luto@...capital.net>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Fenghua Yu <fenghua.yu@...el.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	Oleg Nesterov <oleg@...hat.com>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: question about save_xstate_sig() - WHY DOES THIS WORK?

While working on a patch series to defer FPU state loading until
kernel -> user space transition, and be more lazy with FPU state
while in the kernel, I came across this code in save_xstate_sig().

Not only is this broken with my new code, but it looks like it may
be broken with the current code, too...

Specifically, save_user_xstate() may page fault and sleep. After
returning from the page fault, there is no guarantee that the
FPU state will be restored into the CPU, when the system is not
running with eager fpu mode.

In that case, what prevents us from saving random FPU register state
to the user's stack frame?  Potentially state containing data from
other programs...

        if (user_has_fpu()) {
                /* Save the live register state to the user directly. */
                if (save_user_xstate(buf_fx))
                        return -1;
                /* Update the thread's fxstate to save the fsave header. */
                if (ia32_fxstate)
                        fpu_fxsave(&tsk->thread.fpu);
        } else {
                sanitize_i387_state(tsk);
                if (__copy_to_user(buf_fx, xsave, xstate_size))
                        return -1;
        }

Is this code safe for some reason I have overlooked?

If not, should I post the patch turning the above into "save FPU
state atomically, then copy it to user space" independently from
my optimizations, and submit it for inclusion in -stable as well?
--
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/