linux-kernel - Re: [PATCH 1/3] coredump: flush the fpu exit state for proper multi-threaded core dump

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: <1336693684.1153.30.camel@sbsiddha-desk.sc.intel.com>
Date:	Thu, 10 May 2012 16:48:04 -0700
From:	Suresh Siddha <suresh.b.siddha@...el.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Oleg Nesterov <oleg@...hat.com>, hpa@...or.com, mingo@...e.hu,
	linux-kernel@...r.kernel.org, suresh@...stanetworks.com,
	dhowells@...hat.com, yasutake.koichi@...panasonic.com,
	benh@...nel.crashing.org, paulus@...ba.org, lethal@...ux-sh.org,
	chris@...kel.net
Subject: Re: [PATCH 1/3] coredump: flush the fpu exit state for proper
 multi-threaded core dump

On Thu, 2012-05-10 at 10:04 -0700, Linus Torvalds wrote:
> On Thu, May 10, 2012 at 9:55 AM, Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > My point was, there is no any guarantee prepare_to_copy() does the flush.
> > An architecture can do this in copy_thread() or arch_dup_task_struct(),
> > for example. In fact I do not understand why x86 doesn't do this.
> 
> I agree that it would actually make more sense to do in
> arch_dup_task_struct(). I had trouble finding where the heck the
> fork() code did the FPU fixes back when I was fighting the FPU
> corruption thing.
> 
> The prepare_to_copy() thing is, I think, purely historical, and I
> think we should in fact get rid of it. Everybody else makes it a
> no-op, I think, with a *few* exceptions that seem to have copied the
> x86 model of flushing the FPU there.
> 
> So if somebody sends me a patch to remove that thing, and move the few
> existing users to arch_dup_task_struct(), I'd take it.

ok. Just sent the v2 version of the patchset which includes this. I
didn't compile, test all the architectures. I copied the relevant arch
maintainers who were using prepare_to_copy(). I will be glad if they can
review/test that patch and Ack.

> I think it would be a mistake to use it in the exit path. Make an
> explicit "drop_thread_state()" or similar macro, which can undo FPU
> state and possibly other architecture state.

In my previous/current patchset, I am using arch specific exit_thread()
to call drop_fpu() (changed the name to drop_fpu() from flush_fpu() in
the v2 patchset) during normal exit.

But in the case of coredump, all the other threads are waiting in
exit_mm() as part of the coredump_wait() and this is where we hit the
bug_on, when the main thread finds the other thread is still active on
the runqueue. So based on the discussion with Oleg, I added
wait_task_inactive() check in coredump_wait() to ensure all the other
threads are really off the rq, so that their most recent fpu state is
copied to the memory, which will be copied to the core file by the
dumping thread. Hope this is ok.

thanks,
suresh

--
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/