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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0E8E6C4332F for ; Tue, 7 Nov 2023 21:23:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A03B28000B; Tue, 7 Nov 2023 16:23:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9B3D48D0001; Tue, 7 Nov 2023 16:23:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 87C118000B; Tue, 7 Nov 2023 16:23:19 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 7993C8D0001 for ; Tue, 7 Nov 2023 16:23:19 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 4C97D80B82 for ; Tue, 7 Nov 2023 21:23:19 +0000 (UTC) X-FDA: 81432434118.20.23E9ECC Received: from mail-oo1-f54.google.com (mail-oo1-f54.google.com [209.85.161.54]) by imf22.hostedemail.com (Postfix) with ESMTP id 87D74C0017 for ; Tue, 7 Nov 2023 21:23:17 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aFRY8HKg; spf=pass (imf22.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.161.54 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699392197; a=rsa-sha256; cv=none; b=X+V/qlNTfzJkSHBR1UY5sFoSfjTY/VM55X7656GeW+coYqoLNGGUZ7D/+PqEfoFshz9wDj hj1FjAHWNDbXE/ivFzn8FH//RyN8oinKPpC7dOUpS19v2NByhY1MAxB6YjQG1EPMY6X2fM gCb8LEDXpwQHdjZ1Qk6IzBHVRKgy4io= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aFRY8HKg; spf=pass (imf22.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.161.54 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1699392197; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=p9gkRH7EgBc/EFjx3UGdMuaLkXzoxuUFSfx0n9wI7SU=; b=KJjAzyN0XOZez088MPNXQEoUs9I5eeG5kha+LLS2qKdJ8ODXE7zYMCPF1OyFvKgvBiUmct bGoqdXRIZIhXXFEpiu8DEV7nJHMTgB76Ixc/J8EVhruKptSESdYY3OzFqCk6PMdCV9EjOw 36jtozAJB7ZBe0ktxr9bF6ogSCqRVzU= Received: by mail-oo1-f54.google.com with SMTP id 006d021491bc7-5875c300becso3531597eaf.0 for ; Tue, 07 Nov 2023 13:23:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699392196; x=1699996996; darn=kvack.org; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=p9gkRH7EgBc/EFjx3UGdMuaLkXzoxuUFSfx0n9wI7SU=; b=aFRY8HKgbpsyj9PZZbMpdVRnenQp/i7vAXKPr7S3qeUCGPPsVqvJ57z+iVM3W0Ta9Y 3mgOKs/USpIrhX8zhkqgG2ksAl1PmsSFXbN2ibSdtRr1X7AKARIAMpT4aNcXrpwcCr5M 19A3j5PTOW6+P+eqWc2lOkakMJEEFne6EKOniPWOAHzFUI+VVSn4/Dw/4lMbFkVyhYBn vJU4SHjXRG6CqBJuZUYsleWiXpZjg26e1T9ZIWTrBARAyRnhIqoeC6kRJPx+nmdfkK14 8AAlT3E9oQN0hJ8sC6zLhHZ9wkKiuQsS5AsrY1gt+vYRHGXTNrtG7bP14vUwi9GVp/DL g01Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699392196; x=1699996996; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=p9gkRH7EgBc/EFjx3UGdMuaLkXzoxuUFSfx0n9wI7SU=; b=U030By7qrBWuInXMhP+xTM8Paiq7SwFB4Ar1DqgAp+R7USmwi1f8EDq5s+maxbDA5/ dBBplq7K54njoN0PUXNvKwZy4uhU2WUZLYhJg9OexU9E4qdbl1y85eq3nkh/7q8nVxTs fyL3k2RLMXaGX/Igy0AE4wJmR5mZqV3qfAGmNBldtRt6cGuzrfF7BwMH3wxvABe3qo3s eaUmKE/2wIIjPnLOVVzqfwJ79ijPcJDtykjSQBA3fFlVJj4hg35ShFlZ+rgF7RhgjLNn PAWYGSgnupbfgyeKL9hOs6RGgBE1bwiVlIYrQDjKziDKCTcHKgB+nrHkyVY+ANyTYxBy fEqg== X-Gm-Message-State: AOJu0YwGzjZZNlrOsZuoDyXtin5PejiDbdcO9Dj5HwcbuJ+kN0H2ecPM EiMbRk4xeYxcqEPsy+rTSnSLCFtF+Dx04kHFq60= X-Google-Smtp-Source: AGHT+IH9iCJh9FMWUN07cTY3FUAPQ9BbAXQBdoliTW5WLBxkBSnWDnNDeflruDGHAIyIymjEuK2ubuNKjzaTnXlo4Lw= X-Received: by 2002:a4a:e1d4:0:b0:581:e819:cac5 with SMTP id n20-20020a4ae1d4000000b00581e819cac5mr31277772oot.9.1699392196640; Tue, 07 Nov 2023 13:23:16 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a8a:158f:0:b0:4f0:1250:dd51 with HTTP; Tue, 7 Nov 2023 13:23:16 -0800 (PST) In-Reply-To: <20231107205151.qkwlw7aarjvkyrqs@f> References: <5c7333ea4bec2fad1b47a8fa2db7c31e4ffc4f14.1663334978.git.josh@joshtriplett.org> <202311071228.27D22C00@keescook> <20231107205151.qkwlw7aarjvkyrqs@f> From: Mateusz Guzik Date: Tue, 7 Nov 2023 22:23:16 +0100 Message-ID: Subject: Re: [PATCH] fs/exec.c: Add fast path for ENOENT on PATH search before allocating mm To: Kees Cook Cc: Josh Triplett , Eric Biederman , Alexander Viro , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 87D74C0017 X-Stat-Signature: 3gee6dzze6y69atrw9dusieik5ck8mjw X-Rspam-User: X-HE-Tag: 1699392197-508960 X-HE-Meta: U2FsdGVkX18KmLLiehuITHkz09t+52DugJ3MKjqVw4k3uIxganjUz+dpwkAdq9ZGQXr8BWVStP4JKkbtecy+E/KsH90z3lBXhDc13jNYnhZQEVw2SVQsbII0Q9wPQ8/+u9Jj+2nhpIJ+Gy8nhs60XuVeqLnhWJdhdarLk3qiEB1mxqzuzl1wtEDC7sTGysrNsUPBLU4fMhLRbmrV3eGfxLlV2EPKNTmBAwRIDELtxtFUiGNRPYalYAHCydBuJPwapofjTD/Xe/VeT9t3OzIjbekCnkc079ZEl9ULAVOERBIAE1m5s1aqkn6hRdBVYO1HpYroDHpAnRRJdMalvEWHf8A6ZEWS9FmiDz8UUK6lO/GNw6TE5Ik8uDY/iAMulw81EoiAjJMurZVgg+f6Vs2V8BIPj1RJOI2sOZZoOL6SbRxEJR2r3aXP5fM3zCM66gifUW7GazILo0DnQ4ssgf24L54wDlyJS6JP1bPF1pVHW0ncAWw1uZENa6xvHTO3Wwn4nUSV0otyDDklu7oSnwwfveAlyxxVQfk8q0gh6Il97zhesn4x4pNXzgBNIFRXKDvsEFcENkpGjV1FtvlJSUovaQ/ABcSWbfd/qqZzE0qZu6VwR0eAn6tU1ia0zEcGWRlwcFyoqkzl8hYo/45X0E3tOkZnmKM1dqj1ESOo49wqW1ibU8dcLJB7W0PdwD7xehL5omnzYss6KvWRnubh0DR8I2TMTKIZNV7tiuwIK7T38OD+2xVMeE8D/YiFa/5a6SmiuAJPHdz2qIgODAoD5ta4ZyYbObGCN8D61XPBvw1ZuXQ+k0LR4abq3wCOPoG0XoabyBZSAkWUk+9npeobZHl+eBaYjBj7/ZgLAh6SAcnHrtuA24xb4bAMJ1JBeOYoRxZR5jXycvLVKqUzvqEzWqxIA7rlDQzmy5aWaL9nul0Y7CRQ7yI+37Hw+hHVLCNHTuyE5lvSjIgDNFmwRuKwQh/ puwSKOlC pTufTBGFaNDPc4XwXnR2xzfv4oi3LxlLoAMEnVmPoWsfwa8Lw5HuC9ENUTjTu0MqY/P652yrUFVrBVClpiEWCwfOO5FPlknEZNEWv8IEzxd2LKB5sGCQYY+RXb2xLYHug/sOQT5XLjaQ5K6xoHrqfD8s+m/7L2MDFFhvNiNqBsqOYl6078BAU4u4gpS6o8Zn4N7HQtBng3HG0GHvV7KMevnue6ovC7S0QlSmXmpk4EJvF3KqJGOFQ4Cql565qOGeTqPCHSugAhkhcstCIC0iZO67cxKeEa8nssw8BQ8MWRKmw05CoUOkl78T4Rs2KA+O49npiOORWcPxh/dTH5BerTghdvrvSTKYYTjf2C/zW7C+ElZ80wFB8bnSCgEAf8cL3EZZ7j/LFPXJBBp6Levsji+ZALf5dIG4/8kVFL2Srxrb95vNmIAyHITal651Ak5+vQ00PCcyNawEzsrjekVWOJbNOp8eC8P+ha01Tb/E2zE86ptRxG0m8mZ6LMQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 11/7/23, Mateusz Guzik wrote: > On Tue, Nov 07, 2023 at 12:30:37PM -0800, Kees Cook wrote: >> On Fri, Sep 16, 2022 at 02:41:30PM +0100, Josh Triplett wrote: >> > Currently, execve allocates an mm and parses argv and envp before >> > checking if the path exists. However, the common case of a $PATH search >> > may have several failed calls to exec before a single success. Do a >> > filename lookup for the purposes of returning ENOENT before doing more >> > expensive operations. >> > >> > This does not create a TOCTTOU race, because this can only happen if >> > the >> > file didn't exist at some point during the exec call, and that point is >> > permitted to be when we did our lookup. >> > >> > To measure performance, I ran 2000 fork and execvpe calls with a >> > seven-element PATH in which the file was found in the seventh directory >> > (representative of the common case as /usr/bin is the seventh directory >> > on my $PATH), as well as 2000 fork and execve calls with an absolute >> > path to an existing binary. I recorded the minimum time for each, to >> > eliminate noise from context switches and similar. >> > >> > Without fast-path: >> > fork/execvpe: 49876ns >> > fork/execve: 32773ns >> > >> > With fast-path: >> > fork/execvpe: 36890ns >> > fork/execve: 32069ns >> > >> > The cost of the additional lookup seems to be in the noise for a >> > successful exec, but it provides a 26% improvement for the path search >> > case by speeding up the six failed execs. >> > >> > Signed-off-by: Josh Triplett >> >> *thread necromancy* >> >> I'll snag this patch after -rc1 is out. Based on the research we both >> did in the rest of this thread, this original patch is a clear win. >> Let's get it into linux-next and see if anything else falls out of it. >> > > So the obvious question is why not store lookup result within bprm, > instead of doing the lookup again later. > > Turns out you had very same question and even wrote a patch to sort it > out: https://lore.kernel.org/all/202209161637.9EDAF6B18@keescook/ > > Why do you intend to go with this patch instead? > Welp, should have read the remaining follow up discussion. Even so, the patch which is only doing the lookup once cannot be legitimately slower. Note there is often perf variance between different boots of the same kernel and I suspect this is what justifies it. I would suggest adding a runtime knob to control whether 1. lookups are done late (stock kernel) 2. there is one upfront lookup like in the original patch and finally 3. stuffing file into bprm Then a bunch of runs with the knob cycling between them could serve as a justification. If the patch which dodges second lookup still somehow appears slower a flamegraph or other profile would be nice. I can volunteer to take a look at what's going on provided above measurements will be done and show funkyness. -- Mateusz Guzik