From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:56608 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726122AbeH3V1f (ORCPT ); Thu, 30 Aug 2018 17:27:35 -0400 From: Jeff Layton To: Alexander Viro Cc: "Eric W. Biederman" , =?UTF-8?q?Daniel=20P=20=2E=20Berrang=C3=A9?= , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 0/3] exec: fix passing of file locks across execve in multithreaded processes Date: Thu, 30 Aug 2018 13:24:20 -0400 Message-Id: <20180830172423.21964-1-jlayton@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: v2: fix displaced_files cleanup in __do_execve_file I've done a bit more testing (now with the error handling fixed and it seems to work ok. I have not looked at performance regressions here, as I'm not sure how best to test for that. My main question at this point is whether this is the correct way to fix it. Cover letter from the RFC set follows: -------------------------8<---------------------- A few months ago, Dan reported that when you call execve in process that is multithreaded, any traditional POSIX locks are silently dropped. The problem is that we end up unsharing the files_struct from the process very early during exec, when it looks like it's shared between tasks. Eventually, when the other, non-exec'ing tasks are killed, we tear down the old files_struct. That ends up tearing down the old files_struct, which ends up looking like a close() was issues on each open fd and that causes the locks to be dropped. This patchset is a second stab at fixing this issue, this time following the method suggested by Eric Biederman. The idea here is to move the unshare_files() call after de_thread(), which helps ensure that we only unshare the files_struct when it's truly shared between different processes, and not just when the exec'ing process is multithreaded. This seems to fix the originally reported problem (now known as xfstest generic/484), and basic testing doesn't seem to show any issues. During the original discussion though, Al had mentioned that this could be problematic due to the fdtable being modifiable by other threads (or even processes) during the binfmt probe. That may make this idea non-viable. I'm also not terribly thrilled with the way this sprinkles the files_struct->file_lock all over the place. It may be possible to do some of this with atomic ops if the basic approach turns out to be reasonable. Comments and suggestions welcome. Jeff Layton (3): exec: separate thread_count for files_struct exec: delay clone(CLONE_FILES) if task associated with current files_struct is exec'ing exec: do unshare_files after de_thread fs/exec.c | 25 ++++++++++++++++++------- fs/file.c | 18 ++++++++++++++++++ include/linux/binfmts.h | 1 + include/linux/fdtable.h | 2 ++ kernel/fork.c | 26 ++++++++++++++++++++++---- 5 files changed, 61 insertions(+), 11 deletions(-) -- 2.17.1