linux-kernel - RE: [PATCH] move exit_task_work() before exit

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: <3917C05D9F83184EAA45CE249FF1B1DD0266FC73@SHSMSX103.ccr.corp.intel.com>
Date:	Mon, 29 Dec 2014 01:32:37 +0000
From:	"Ma, Xindong" <xindong.ma@...el.com>
To:	Peter Hurley <peter@...leysoftware.com>,
	Oleg Nesterov <oleg@...hat.com>,
	Al Viro <viro@...IV.linux.org.uk>
CC:	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"mhocko@...e.cz" <mhocko@...e.cz>,
	"mingo@...nel.org" <mingo@...nel.org>,
	"peterz@...radead.org" <peterz@...radead.org>,
	"riel@...hat.com" <riel@...hat.com>,
	"ionut.m.alexa@...il.com" <ionut.m.alexa@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Zhang, Di" <di.zhang@...el.com>,
	"Sun, Zhonghua" <zhonghua.sun@...el.com>
Subject: RE: [PATCH] move exit_task_work() before exit_fs().

> On 12/28/2014 07:58 PM, Ma, Xindong wrote:
> >>
> >> On 12/26, Leon Ma wrote:
> >>>
> >>> We encountered following panic. The scenario is the process is
> >>> exiting and executing its task work. When closing dev node, the
> >>> driver triggers a firmware reload according to device status.
> >>> Because task->fs is
> >> set to NULL in exit_fs(), panic happens.
> >>
> >> I think this should be fixed somewhere else...
> > Yes, for this panic, I also think driver is not perfect and need a fix. But
> kernel should not add the limitation like this...
> >>
> >>> Task work is a common interface, we should not limite the resource
> >>> the
> >> user will utilize.
> >>
> >> Exactly. And note that with this patch exit_mm()..disassociate_ctty()
> >> paths can't use task works.
> > I don't get this. Currently disassociate_ctty() is also called after exit_mm()
> and exit_task_work(). My patch didn't change this.
> 
> ??
> 
> 742-	if (group_dead)
> 743:		disassociate_ctty(1);
> 744-	exit_task_namespaces(tsk);
> 745-	exit_task_work(tsk);
> 746-	exit_thread();
Sorry, I was checking the old source code when applying this mail. I did a check on latest code, and did not find disassociate_ctty() was using task work. So why this matters?
> 
> >> Not to mention that this patch moves exit_files() up, even before
> >> exit_mm(), without any explanation.
> > Moving exit_files() up is because exit_files() closes files and add tasks to
> task works.
> >>
> >> Add Al. May be we can move exit_fs() down after exit_task_work(), I
> >> dunno, but to me it would be better to change the driver.
> >>
> > I'm OK with this suggestion to fix this issue. I'm not sure whether in the
> future task work users will access other resources and expose other issues.
> >