From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: ebiederm@xmission.com (Eric W. Biederman) To: Oleg Nesterov Cc: Jeff Layton , viro@zeniv.linux.org.uk, berrange@redhat.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Morton References: <20180914105310.6454-1-jlayton@kernel.org> <20180914105310.6454-2-jlayton@kernel.org> <20180915160423.GA31461@redhat.com> <87o9cxv2wh.fsf@xmission.com> <20180917152424.GA25173@redhat.com> Date: Mon, 17 Sep 2018 22:45:09 +0200 In-Reply-To: <20180917152424.GA25173@redhat.com> (Oleg Nesterov's message of "Mon, 17 Sep 2018 17:24:24 +0200") Message-ID: <87d0tbn99m.fsf@xmission.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [PATCH v3 1/3] exec: separate thread_count for files_struct Sender: linux-kernel-owner@vger.kernel.org List-ID: Oleg Nesterov writes: > On 09/16, Eric W. Biederman wrote: >> >> Oleg Nesterov writes: >> >> > As for binder.c, in this case we probably actually want to unshare ->files >> > on exec so we can ignore it? >> >> Looking at the binder case it only captures ->files on mmap. Exec >> ditches the mmap. So if the order of operations are correct than >> the dropping of the old mm will also drop the count on files_struct >> held by binder. >> >> So semantically binder should not effect locks on exec, > > Agreed, but it does. > > Before your "[PATCH 0/3] exec: Moving unshare_files_struct" unshare_files() > is called before exec_mmap(). > > And even with this series we can have another CLONE_VM process. > > Howver, I think this doesn't really matter. binder does __fd_install(files), > so if it actually has a reference to execing_task->files, I think it should > be unshared anyway. > >> In short as long as we get the oder of operations correct we should be >> able to safely ignore binder, and not have binder affect the results of >> this code. > > Agreed. I may have spoken too soon. Binder uses schedule_work to call put_files_struct from munmap. So the files->count may still be elevated after the mm is put. Ick. Eric