From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:38562 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727727AbeIOV4j (ORCPT ); Sat, 15 Sep 2018 17:56:39 -0400 Date: Sat, 15 Sep 2018 18:37:04 +0200 From: Oleg Nesterov To: Jeff Layton Cc: viro@zeniv.linux.org.uk, ebiederm@xmission.com, berrange@redhat.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Morton Subject: Re: [PATCH v3 3/3] exec: do unshare_files after de_thread Message-ID: <20180915163704.GA31693@redhat.com> References: <20180914105310.6454-1-jlayton@kernel.org> <20180914105310.6454-4-jlayton@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180914105310.6454-4-jlayton@kernel.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 09/14, Jeff Layton wrote: > > POSIX mandates that open fds and their associated file locks should be > preserved across an execve. This works, unless the process is > multithreaded at the time that execve is called. > > In that case, we'll end up unsharing the files_struct but the locks will > still have their fl_owner set to the address of the old one. Eventually, > when the other threads die and the last reference to the old > files_struct is put, any POSIX locks get torn down since it looks like > a close occurred on them. > > The result is that all of your open files will be intact with none of > the locks you held before execve. The simple answer to this is "use OFD > locks", but this is a nasty surprise and it violates the spec. > > Fix this by doing unshare_files later during exec, See my reply to 1/3... if we can forget about the races with get_files_struct() we can probably make a much simpler patch, plus we do not need 2/2, afaics. What I really can't understand is why we need to _change_ current->files early in do_execve(). IOW. Lets ignore do_close_on_exec(), lets ignore the fact that unshare_fd() can fail and thus it makes sense to call it before point-of-no-return. Any other reason why we can't simply call unshare_files() at the end of __do_execve_file() on success? Oleg.