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=-5.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 316A7C2D0A3 for ; Mon, 2 Nov 2020 20:31:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C954722245 for ; Mon, 2 Nov 2020 20:31:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=kernel-dk.20150623.gappssmtp.com header.i=@kernel-dk.20150623.gappssmtp.com header.b="vLvy5N3q" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726719AbgKBUbL (ORCPT ); Mon, 2 Nov 2020 15:31:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49344 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725801AbgKBUbK (ORCPT ); Mon, 2 Nov 2020 15:31:10 -0500 Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 60E5AC0617A6 for ; Mon, 2 Nov 2020 12:31:10 -0800 (PST) Received: by mail-pf1-x443.google.com with SMTP id y14so12137593pfp.13 for ; Mon, 02 Nov 2020 12:31:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=taI5V1PmKR84k5WuxRX//AHvUGNpKhqq+imLNWnt1k4=; b=vLvy5N3qjQFUvEJJO5v5PRIQdTgbNoxDRQCor83tF0Uqv19vCQKM2Wcz4NIjY84WaP ZDuhPcrFM5YZvvGHOFRaVOYe2BkrLh5jZzhkpzt8G2bLTi1SpSSFbvqzi5noI8vmGITL rKpJxFQLBvb4yb4IxCyZgSH+w0R/pfttrVvoKqx9wEp+1drmpUchYbncawehUDXxAj+S I0D8zRVbkofR+b8Ne3DkQVRkrDZBJeF1csXcB5rWr4bdXIFLjh6xzmzui8jsduIRDNRH Amml1LTu86cyl1k8s/AP2lrjEIP3R04L2h7J7OkC5hBvzIduhZQYMNXeRagDjIBuaO5I iaMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=taI5V1PmKR84k5WuxRX//AHvUGNpKhqq+imLNWnt1k4=; b=Qyv/0Gd5SzPuY7CSPsDSCikXM7k4UZzAYsZtVvViviGjrZ191jgSi15ppU3+5nUXUg rOo/S0IDxVyrjC6SmNvMUVrgogTPGACweyTT0iWqB4XYMeVfPaK/fHXmyGpuRAw864cY u8Ync9EYIxmGMdY7xlBBLzcykJft1jFDvV1SfXZQ0kBRRQrcctNUCsiiKuXo0zDdHiNa ulyQ1y/VdJNfmctaso79ia8aLlmUDt7HqPwkWKoiLKmuiperImhTyMvvCmYBoxdAtmm7 YiyNTvpetuGKmk16kbct1YlspNRkS1P/xil9uKXudcCfioZtzo9w9L7hWrJzmSbIz2xB RDJA== X-Gm-Message-State: AOAM533w7SCFEKrPpGUjnS42ZKOU6suHHIHMZo9mkklXWMyJfAhGO+qM 4VnokyBNdE4TsfnCQegMtp0DAlwI6CrQ9A== X-Google-Smtp-Source: ABdhPJwd9hKXh1kE1Bcx0wezEg07bK53IiR4nPRLnnPmcSCDUsKuKPiMakIXHBOfzj21lTyawQMk2w== X-Received: by 2002:a17:90b:4749:: with SMTP id ka9mr19576253pjb.197.1604349069502; Mon, 02 Nov 2020 12:31:09 -0800 (PST) Received: from [192.168.1.134] ([66.219.217.173]) by smtp.gmail.com with ESMTPSA id e13sm5210784pfm.2.2020.11.02.12.31.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 02 Nov 2020 12:31:08 -0800 (PST) Subject: Re: [PATCH -next] fs: Fix memory leaks in do_renameat2() error paths To: "Eric W. Biederman" 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> <87k0v38qlw.fsf@x220.int.ebiederm.org> From: Jens Axboe Message-ID: Date: Mon, 2 Nov 2020 13:31:07 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <87k0v38qlw.fsf@x220.int.ebiederm.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/2/20 1:12 PM, Eric W. Biederman wrote: > 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. Thanks Eric, that's useful. Sounds to me like we're better off, at least for now, to just expressly forbid async lookup of /proc/self/. Which isn't really the end of the world as far as I'm concerned. -- Jens Axboe