From patchwork Mon Aug 27 17:47:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 10577439 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B39381805 for ; Mon, 27 Aug 2018 17:47:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A5FD226246 for ; Mon, 27 Aug 2018 17:47:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 99D792837D; Mon, 27 Aug 2018 17:47:31 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 46CDB26246 for ; Mon, 27 Aug 2018 17:47:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727053AbeH0VfD (ORCPT ); Mon, 27 Aug 2018 17:35:03 -0400 Received: from mail.kernel.org ([198.145.29.99]:60152 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726958AbeH0VfD (ORCPT ); Mon, 27 Aug 2018 17:35:03 -0400 Received: from tleilax.poochiereds.net (cpe-71-70-156-158.nc.res.rr.com [71.70.156.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B95D42088E; Mon, 27 Aug 2018 17:47:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1535392049; bh=stasJp9NoRm2uUmjKlaG4njv3WYZRCReSVBHHNz7q+M=; h=From:To:Cc:Subject:Date:From; b=olAyg7ibxoye5b/cbcWOSMbcS48PH+Kpzy00WvYH1PF+oIGUzv8ZKup9u6qp0BT5J JPy3GzaDIPgGiRY3CKLg7pNZthVzHRko3v6nVSEppBKPUStd6w886DwewZVfYseTsz l2lnJio97ykyBuyFbq2K20gYd6oMhG5ijWzwqWpM= From: Jeff Layton To: Alexander Viro Cc: "Eric W. Biederman" , =?utf-8?q?Daniel_P_=2E_Berr?= =?utf-8?q?ang=C3=A9?= , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [RFC PATCH 0/3] exec: fix passing of file locks across execve in multithreaded processes Date: Mon, 27 Aug 2018 13:47:19 -0400 Message-Id: <20180827174722.3723-1-jlayton@kernel.org> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 | 29 +++++++++++++++++++---------- fs/file.c | 18 ++++++++++++++++++ include/linux/binfmts.h | 1 + include/linux/fdtable.h | 2 ++ kernel/fork.c | 26 ++++++++++++++++++++++---- 5 files changed, 62 insertions(+), 14 deletions(-)