From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2E99C00A89 for ; Mon, 2 Nov 2020 20:13:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A3E3F21D40 for ; Mon, 2 Nov 2020 20:13:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727074AbgKBUNy (ORCPT ); Mon, 2 Nov 2020 15:13:54 -0500 Received: from out03.mta.xmission.com ([166.70.13.233]:56652 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726794AbgKBUND (ORCPT ); Mon, 2 Nov 2020 15:13:03 -0500 Received: from in01.mta.xmission.com ([166.70.13.51]) by out03.mta.xmission.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.93) (envelope-from ) id 1kZgCK-00512W-VY; Mon, 02 Nov 2020 13:13:01 -0700 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1kZgCK-0003Wq-0p; Mon, 02 Nov 2020 13:13:00 -0700 From: ebiederm@xmission.com (Eric W. Biederman) To: Jens Axboe Cc: Al Viro , Qian Cai , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org References: <20201030152407.43598-1-cai@redhat.com> <20201030184255.GP3576660@ZenIV.linux.org.uk> <20201030184918.GQ3576660@ZenIV.linux.org.uk> <20201030222213.GR3576660@ZenIV.linux.org.uk> <87eelba7ai.fsf@x220.int.ebiederm.org> Date: Mon, 02 Nov 2020 14:12:59 -0600 In-Reply-To: (Jens Axboe's message of "Mon, 2 Nov 2020 12:54:40 -0700") Message-ID: <87k0v38qlw.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1kZgCK-0003Wq-0p;;;mid=<87k0v38qlw.fsf@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/uzcPHiTMRr+f9d2gG7BAWgMjV5uEet8c= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH -next] fs: Fix memory leaks in do_renameat2() error paths X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Jens Axboe writes: > On 11/2/20 12:27 PM, Eric W. Biederman wrote: >> Jens Axboe writes: >> >>> On 10/30/20 4:22 PM, Al Viro wrote: >>>> On Fri, Oct 30, 2020 at 02:33:11PM -0600, Jens Axboe wrote: >>>>> On 10/30/20 12:49 PM, Al Viro wrote: >>>>>> On Fri, Oct 30, 2020 at 12:46:26PM -0600, Jens Axboe wrote: >>>>>> >>>>>>> See other reply, it's being posted soon, just haven't gotten there yet >>>>>>> and it wasn't ready. >>>>>>> >>>>>>> It's a prep patch so we can call do_renameat2 and pass in a filename >>>>>>> instead. The intent is not to have any functional changes in that prep >>>>>>> patch. But once we can pass in filenames instead of user pointers, it's >>>>>>> usable from io_uring. >>>>>> >>>>>> You do realize that pathname resolution is *NOT* offloadable to helper >>>>>> threads, I hope... >>>>> >>>>> How so? If we have all the necessary context assigned, what's preventing >>>>> it from working? >>>> >>>> Semantics of /proc/self/..., for starters (and things like /proc/mounts, etc. >>>> *do* pass through that, /dev/stdin included) >>> >>> Don't we just need ->thread_pid for that to work? >> >> No. You need ->signal. >> >> You need ->signal->pids[PIDTYPE_TGID]. It is only for /proc/thread-self >> that ->thread_pid is needed. >> >> Even more so than ->thread_pid, it is a kernel invariant that ->signal >> does not change. > > I don't care about the pid itself, my suggestion was to assign ->thread_pid > over the lookup operation to ensure that /proc/self/ worked the way that > you'd expect. I understand that. However /proc/self/ refers to the current process not to the current thread. So ->thread_pid is not what you need to assign to make that happen. What the code looks at is: ->signal->pids[PIDTYPE_TGID]. It will definitely break invariants to assign to ->signal. Currently only exchange_tids assigns ->thread_pid and it is nasty. It results in code that potentially results in infinite loops in kernel/signal.c To my knowledge nothing assigns ->signal->pids[PIDTYPE_TGID]. At best it might work but I expect the it would completely confuse something in the pid to task or pid to process mappings. Which is to say even if it does work it would be an extremely fragile solution. Eric