Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     Oleg Nesterov <oleg@redhat.com>
Cc:     Johannes Weiner <hannes@cmpxchg.org>,
        Michal Hocko <mhocko@kernel.org>,
        Kirill Tkhai <ktkhai@virtuozzo.com>, akpm@linux-foundation.org,
        peterz@infradead.org, viro@zeniv.linux.org.uk, mingo@kernel.org,
        paulmck@linux.vnet.ibm.com, keescook@chromium.org, riel@redhat.com,
        tglx@linutronix.de, kirill.shutemov@linux.intel.com,
        marcos.souza.org@gmail.com, hoeun.ryu@gmail.com,
        pasha.tatashin@oracle.com, gs051095@gmail.com, dhowells@redhat.com,
        rppt@linux.vnet.ibm.com, linux-kernel@vger.kernel.org,
        Balbir Singh <balbir@linux.vnet.ibm.com>,
        Tejun Heo <tj@kernel.org>
References: <20180503133338.GA23401@redhat.com> <87y3h0x0qg.fsf@xmission.com>
        <20180504142056.GA26151@redhat.com> <87r2mrh4is.fsf@xmission.com>
        <20180504145435.GA26573@redhat.com> <87y3gzfmjt.fsf@xmission.com>
        <20180504162209.GB26573@redhat.com> <871serfk77.fsf@xmission.com>
        <20180507143358.GA3071@redhat.com> <87vabyvnw0.fsf@xmission.com>
        <20180509144016.GA25742@redhat.com> <87vabwp5p6.fsf@xmission.com>
Date:   Wed, 09 May 2018 23:03:23 -0500
In-Reply-To: <87vabwp5p6.fsf@xmission.com> (Eric W. Biederman's message of
        "Wed, 09 May 2018 22:09:25 -0500")
Message-ID: <871sekp378.fsf_-_@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: [RFC][PATCH] cgroup: Don't mess with tasks in exec
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Semantically exec is supposed to be atomic with no user space visible
intermediate points. Migrating tasks during exec may change that and
lead to all manner of difficult to analyze and maintin corner cases.

So avoid the problems by simply blocking cgroup migration over the
entirety of exec.

Reported-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---

Unless this leads to some kind of deadlock 
 fs/exec.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 32461a1543fc..54bb01cfc635 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1101,7 +1099,6 @@ static int de_thread(struct task_struct *tsk)
 		struct task_struct *leader = tsk->group_leader;
 
 		for (;;) {
-			cgroup_threadgroup_change_begin(tsk);
 			write_lock_irq(&tasklist_lock);
 			/*
 			 * Do this under tasklist_lock to ensure that
@@ -1112,7 +1109,6 @@ static int de_thread(struct task_struct *tsk)
 				break;
 			__set_current_state(TASK_KILLABLE);
 			write_unlock_irq(&tasklist_lock);
-			cgroup_threadgroup_change_end(tsk);
 			schedule();
 			if (unlikely(__fatal_signal_pending(tsk)))
 				goto killed;
@@ -1750,6 +1746,7 @@ static int do_execveat_common(int fd, struct filename *filename,
 	if (retval)
 		goto out_free;
 
+	cgroup_threadgroup_change_begin(current);
 	check_unsafe_exec(bprm);
 	current->in_execve = 1;
 
@@ -1822,6 +1819,7 @@ static int do_execveat_common(int fd, struct filename *filename,
 	/* execve succeeded */
 	current->fs->in_exec = 0;
 	current->in_execve = 0;
+	cgroup_threadgroup_change_end(current);
 	membarrier_execve(current);
 	acct_update_integrals(current);
 	task_numa_free(current);
@@ -1841,6 +1839,7 @@ static int do_execveat_common(int fd, struct filename *filename,
 out_unmark:
 	current->fs->in_exec = 0;
 	current->in_execve = 0;
+	cgroup_threadgroup_change_end(current);
 
 out_free:
 	free_bprm(bprm);
-- 
2.14.1