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 40CF3CCA479 for ; Mon, 4 Jul 2022 15:49:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A4A026B0072; Mon, 4 Jul 2022 11:49:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9F9B76B0073; Mon, 4 Jul 2022 11:49:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8C1536B0074; Mon, 4 Jul 2022 11:49:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 79E466B0072 for ; Mon, 4 Jul 2022 11:49:53 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay13.hostedemail.com (Postfix) with ESMTP id 362F660571 for ; Mon, 4 Jul 2022 15:49:52 +0000 (UTC) X-FDA: 79649853024.22.6C0EAB9 Received: from mail-yw1-f170.google.com (mail-yw1-f170.google.com [209.85.128.170]) by imf04.hostedemail.com (Postfix) with ESMTP id D3EDC40004 for ; Mon, 4 Jul 2022 15:49:50 +0000 (UTC) Received: by mail-yw1-f170.google.com with SMTP id 00721157ae682-317a66d62dfso85760487b3.7 for ; Mon, 04 Jul 2022 08:49:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=kB0Q9EykRhYRQy5w/R9rD3NHZHzS133LEDTFMEV0gnQ=; b=BsVN/T+Cr7Fz7QRKx37MaIbmzPtOjRBSjLtgtEvQCniQ32QrFtiF8UfJhCBYYXZ05D c1clpwBbV8do3uOIfULrsIyFme7FKlxhhNloVhnb0WCwGQCsqZL2LaWdT59oDEHsZiGE kJpwSABagBj7urC61V/vDuT612FfRVtv3YAkF4k6oB4VDywd/VNccUpmzeIln3Z2w5pj 6NhMc/VheMFYI03tORin+p0Ph1kBtJoa0rnZzXDoc3HKJZVJVAmsz7z8lXw2HGJuULGy G/ZEEkMinCjHEVeBuWXZaqX/7tYOiDhQVk7CgN/nSOCGQ3PUfhlhdXzqXu7VtXkec5yK 7i9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=kB0Q9EykRhYRQy5w/R9rD3NHZHzS133LEDTFMEV0gnQ=; b=LxzU6VsvVU494lqenODopdHhEgYKnQAb2Rez/qVYunaIvvLVCdx2lJfckg5xoUH6SY aD2clE95NF1m2t9KFzVLhszjvmuPxQjN/38p3SeaUN4sk9TZ67VPa2v2VeKqIMCVf4dQ HiadPvOh5NbRZNWiFZnsjrnlcnFXE/00Q4/R/oCnVWrp0jPoym/CF/DaB+rak1/LVfqC Wtq3baBw1rIS5P6zV1ySMhV7x7uAoyj0fudHRmloadYkdg0OqVniSF/ee3zUM2pIGMto OIgoWc/+2W21uZimiFeIvq3SSoWPDT67I1UlFLIKTmcfcCYvUurQY6bbyxTjdDYv7Yhg gqFw== X-Gm-Message-State: AJIora9sxabyghhzRzhybVW0Y4viyQxNWza7QwngyBIjbKghxsbA5+lh c4Uts7BnDXGhLoVhrZZoSCzPPi5yEiceFpWxrXhxlw== X-Google-Smtp-Source: AGRyM1tiVL3OesiKZ3ynhuSHFjuYYA7A2DN8MA5Tl6Wm/gnSAWj/xnfNXvXMzmfAODFHhl+OtNE0xkrQCcqeO6a8LnQ= X-Received: by 2002:a81:a847:0:b0:31c:7dd5:6d78 with SMTP id f68-20020a81a847000000b0031c7dd56d78mr15737307ywh.50.1656949789965; Mon, 04 Jul 2022 08:49:49 -0700 (PDT) MIME-Version: 1.0 References: <20220701142310.2188015-1-glider@google.com> <20220701142310.2188015-44-glider@google.com> In-Reply-To: From: Alexander Potapenko Date: Mon, 4 Jul 2022 17:49:13 +0200 Message-ID: Subject: Re: [PATCH v4 43/45] namei: initialize parameters passed to step_into() To: Al Viro Cc: Linus Torvalds , Alexei Starovoitov , Andrew Morton , Andrey Konovalov , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Christoph Hellwig , Christoph Lameter , David Rientjes , Dmitry Vyukov , Eric Dumazet , Greg Kroah-Hartman , Herbert Xu , Ilya Leoshkevich , Ingo Molnar , Jens Axboe , Joonsoo Kim , Kees Cook , Marco Elver , Mark Rutland , Matthew Wilcox , "Michael S. Tsirkin" , Pekka Enberg , Peter Zijlstra , Petr Mladek , Steven Rostedt , Thomas Gleixner , Vasily Gorbik , Vegard Nossum , Vlastimil Babka , kasan-dev , Linux-MM , linux-arch , Linux Kernel Mailing List , Evgenii Stepanov , Nathan Chancellor , Nick Desaulniers , Segher Boessenkool , Vitaly Buka , linux-toolchains Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="BsVN/T+C"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of glider@google.com designates 209.85.128.170 as permitted sender) smtp.mailfrom=glider@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1656949790; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=kB0Q9EykRhYRQy5w/R9rD3NHZHzS133LEDTFMEV0gnQ=; b=pk3WfRPNo+hBmee1Y3hJZMrI75VCu0CBLvWuKo89Y99md/nwSZ89IfY+rgS2mq93DkRYow /vUDTuxRXVo1neqJFbRCTuAuadleU+nY5mrRx+b0BApwj+I9xx8qwCm9zxCXuQs02/QG30 wATS3boY3SVBQ6pk3IM7cGXj102L6es= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1656949790; a=rsa-sha256; cv=none; b=N5+fJA1DPhmnlW+3KGJQc9vtZqq8At2DOR+jsXWAzXmYib58H5b94lywry3JTDXXZaASZj ljqKzFzt2tjSQXDVnXjTIHMkYyrHV1nIeEHZHLKqE3WqZjWEymkoWIQPCIq4ZKcEApojkD tGQUbFcuynLoZZlq7bPfzi9Wuv1noOQ= X-Stat-Signature: 8rzdrftw47k85fb593qto7m5x5kd55gj X-Rspamd-Queue-Id: D3EDC40004 Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="BsVN/T+C"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of glider@google.com designates 209.85.128.170 as permitted sender) smtp.mailfrom=glider@google.com X-Rspamd-Server: rspam03 X-Rspam-User: X-HE-Tag: 1656949790-515689 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Jul 4, 2022 at 3:44 PM Al Viro wrote: > > On Mon, Jul 04, 2022 at 10:20:53AM +0200, Alexander Potapenko wrote: > > > What makes you think they are false positives? Is the scenario I > > described above: > > > > """ > > In particular, if the call to lookup_fast() in walk_component() > > returns NULL, and lookup_slow() returns a valid dentry, then the > > `seq` and `inode` will remain uninitialized until the call to > > step_into() > > """ > > > > impossible? > > Suppose step_into() has been called in non-RCU mode. The first > thing it does is > int err =3D handle_mounts(nd, dentry, &path, &seq); > if (err < 0) > return ERR_PTR(err); > > And handle_mounts() in non-RCU mode is > path->mnt =3D nd->path.mnt; > path->dentry =3D dentry; > if (nd->flags & LOOKUP_RCU) { > [unreachable code] > } > [code not touching seqp] > if (unlikely(ret)) { > [code not touching seqp] > } else { > *seqp =3D 0; /* out of RCU mode, so the value doesn't mat= ter */ > } > return ret; > > In other words, the value seq argument of step_into() used to have ends u= p > being never fetched and, in case step_into() gets past that if (err < 0) > that value is replaced with zero before any further accesses. Oh, I see. That is actually what had been discussed here: https://lore.kernel.org/linux-toolchains/20220614144853.3693273-1-glider@go= ogle.com/ Indeed, step_into() in its current implementation does not use `seq` (which is noted in the patch description ;)), but the question is whether we want to catch such cases regardless of that. One of the reasons to do so is standard compliance - passing an uninitialized value to a function is UB in C11, as Segher pointed out here: https://lore.kernel.org/linux-toolchains/20220614214039.GA25951@gate.= crashing.org/ The compilers may not be smart enough to take advantage of this _yet_, but I wouldn't underestimate their ability to evolve (especially that of Clang). I also believe it's fragile to rely on the callee to ignore certain parameters: it may be doing so today, but if someone changes step_into() tomorrow we may miss it. If I am reading Linus's message here (and the following one from him in the same thread): https://lore.kernel.org/linux-toolchains/CAHk-=3Dwhjz3wO8zD+itoerphWem+JZz4= uS3myf6u1Wd6epGRgmQ@mail.gmail.com/ correctly, we should be reporting uninitialized values passed to functions, unless those values dissolve after inlining. While this is a bit of a vague criterion, at least for Clang we always know that KMSAN instrumentation is applied after inlining, so the reports we see are due to values that are actually passed between functions. > So it's a false positive; yes, strictly speaking compiler is allowd > to do anything whatsoever if it manages to prove that the value is > uninitialized. Realistically, though, especially since unsigned int > is not allowed any trapping representations... > > If you want an test stripped of VFS specifics, consider this: > > int g(int n, _Bool flag) > { > if (!flag) > n =3D 0; > return n + 1; > } > > int f(int n, _Bool flag) > { > int x; > > if (flag) > x =3D n + 2; > return g(x, flag); > } > > Do your tools trigger on it? Currently KMSAN has two modes of operation controlled by CONFIG_KMSAN_CHECK_PARAM_RETVAL. When enabled, that config enforces checks of function parameters at call sites (by applying Clang's -fsanitize-memory-param-retval flag). In that mode the tool would report the attempt to call `g(x, false)` if g() survives inlining. In the case CONFIG_KMSAN_CHECK_PARAM_RETVAL=3Dn, KMSAN won't be reporting the error. Based on the mentioned discussion I decided to make CONFIG_KMSAN_CHECK_PARAM_RETVAL=3Dy the default option. So far it only reported a handful of errors (i.e. enforcing this rule shouldn't be very problematic for the kernel), but it simplifies handling of calls between instrumented and non-instrumented functions that occur e.g. at syscall entry points: knowing that passed-by-value arguments are checked at call sites, we can assume they are initialized in the callees. HTH, Alex --=20 Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Stra=C3=9Fe, 33 80636 M=C3=BCnchen Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Falls Sie diese f=C3=A4lschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, l=C3=B6schen Sie alle Kopien und Anh=C3=A4nge davon und lassen Sie = mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde. This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.