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 97A52C43334 for ; Fri, 10 Jun 2022 00:33:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 271B58D006B; Thu, 9 Jun 2022 20:33:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 220C08D0064; Thu, 9 Jun 2022 20:33:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0C2228D006B; Thu, 9 Jun 2022 20:33:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id EDF1D8D0064 for ; Thu, 9 Jun 2022 20:33:00 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id BCA1B216B7 for ; Fri, 10 Jun 2022 00:33:00 +0000 (UTC) X-FDA: 79560451320.16.72D3EEA Received: from mail-lf1-f45.google.com (mail-lf1-f45.google.com [209.85.167.45]) by imf21.hostedemail.com (Postfix) with ESMTP id 59FBB1C007E for ; Fri, 10 Jun 2022 00:33:00 +0000 (UTC) Received: by mail-lf1-f45.google.com with SMTP id a15so40436267lfb.9 for ; Thu, 09 Jun 2022 17:33:00 -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; bh=M1G6Hf9v6f6xyLP2a6Gh7yfuFJ+6ipgBJHHWwU80v1c=; b=GzLaviOqRCp4B7LqsBfA2qFyX6V1uMUAbIpS5ZG+ymr/9aN/hsQn1QW3PtN+GmgInQ He0Q1Uquu8aHzwNdzglLiDyax/ofVj35MhNvH0zBU7U/FgsC8lV4WZcV1MRSavA6givw MwwoH6cUS805gA1B/kna/OiRrljxuadDNbWQe2uk3jqSGIPZTj1Nggn2aFmaJOxpn0ee keyXmIa2x54GZzES+MYP7V0JK7Y0RzKUk9jyYpIawwn1bBhBPn8bT+rMBalAXRhKRGqN ws66+mukc8/rRLkWY5c5tACdnnEguXj7+LFVYthISMqK0wDGsAxLLnOu3EGgFefxrZxd +Qcg== 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; bh=M1G6Hf9v6f6xyLP2a6Gh7yfuFJ+6ipgBJHHWwU80v1c=; b=NhAwkpPKI6toiA7psGxJkHIABkpagB/l/zY6VRnVHOTmNmBTRe/7RPuijisgYKYhuq ULTD93VSdXO0qE7MvYy9nYb78w1rOjoEKhvTFEGSVM4lzOBOKXqM8+m/bnBU52H2qIUa LMaZkvmVbFScUAAieR2lgp59U+B68Ncs7/mZULypxTq7t7ARvubk5FOtKRQWR3TCUo9X su4iSXZyfCxUlIqpN2LzHhcaOKr2eYyOVevVdlRit+rLdnHSVXEzKX3TXrCujFBihu8+ HoTlfqNt5CFfKNZ50GO9Zt8DuagFjH0B+N8rEW7XSWUqeai3Ktj2WNK6Z74vhnXJAbyO GEmw== X-Gm-Message-State: AOAM533/h1JLrByCjDMpIs1C5i5K3aVO08ISwUbD988s/vRSo6NrrJUJ VHiRI8qb4IR4FH0WDMclS63YasNpyusRwQvvFW35bQ== X-Google-Smtp-Source: ABdhPJwmAai5PXOj7tGhLdM9ZQqNV5Wv1qkbCkukxf252/r1aHu4spL4q3u9asT4a5t/dEsKFtmgLChgmJAimWPi9do= X-Received: by 2002:a05:6512:ad6:b0:479:5599:d834 with SMTP id n22-20020a0565120ad600b004795599d834mr11951775lfu.103.1654821178297; Thu, 09 Jun 2022 17:32:58 -0700 (PDT) MIME-Version: 1.0 References: <20220609221702.347522-1-morbo@google.com> <20220609152527.4ad7862d4126e276e6f76315@linux-foundation.org> In-Reply-To: <20220609152527.4ad7862d4126e276e6f76315@linux-foundation.org> From: Nick Desaulniers Date: Thu, 9 Jun 2022 17:32:46 -0700 Message-ID: Subject: Re: [PATCH 00/12] Clang -Wformat warning fixes To: Andrew Morton , Bill Wendling , Randy Dunlap Cc: Bill Wendling , Tony Luck , Borislav Petkov , Thomas Gleixner , Ingo Molnar , Dave Hansen , "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" , "H. Peter Anvin" , Phillip Potter , Arnd Bergmann , Greg Kroah-Hartman , "Rafael J. Wysocki" , Jan Kara , Pablo Neira Ayuso , Jozsef Kadlecsik , Florian Westphal , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jaroslav Kysela , Takashi Iwai , Nathan Chancellor , Tom Rix , Ross Philipson , Daniel Kiper , linux-edac@vger.kernel.org, LKML , ACPI Devel Maling List , Linux Memory Management List , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, Network Development , alsa-devel@alsa-project.org, clang-built-linux , Justin Stitt , Justin Stitt Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1654821180; a=rsa-sha256; cv=none; b=A/Py7IDHLc3l3p/JZL5A+zWdV96HVS0i0PfyaGK11E/2JkwGlEz1hkoyCQEWsVozxL3Jwl o98accm/R38PxGuswsUFt/sd/i5PQ8cVGYTvGenW3gj/iOytubxsxUMg1pGN2/Ms4Cfc/s VtTXKSqf1LprdPJwJC2tk+IZ9qS4+6s= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=GzLaviOq; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of ndesaulniers@google.com designates 209.85.167.45 as permitted sender) smtp.mailfrom=ndesaulniers@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1654821180; 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=M1G6Hf9v6f6xyLP2a6Gh7yfuFJ+6ipgBJHHWwU80v1c=; b=w90nDSXYMeYgkB8KgfWBeze+ZNWfhJ1PX/Fye0q/VpNAX5tWk4UCBKCG2XQLfHZCeP8zDx 4w5r1aZLruwLw5TpI7mcl2hrWqLRZ2btMlKGBwxWVinu0dVdto2h6lf80Eqf6rFHv6Dk9w KFylxCz0MlENphuegwWY6mcN0qmj9cQ= X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 59FBB1C007E Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=GzLaviOq; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of ndesaulniers@google.com designates 209.85.167.45 as permitted sender) smtp.mailfrom=ndesaulniers@google.com X-Stat-Signature: oedu8789kbyattzqweormq3m5t6cqds1 X-HE-Tag: 1654821180-363464 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 Thu, Jun 9, 2022 at 3:25 PM Andrew Morton wrote: > > On Thu, 9 Jun 2022 22:16:19 +0000 Bill Wendling wrote: > > > This patch set fixes some clang warnings when -Wformat is enabled. It looks like this series fixes -Wformat-security, which while being a member of the -Wformat group, is intentionally disabled in the kernel and somewhat orthogonal to enabling -Wformat with Clang. -Wformat is a group flag (like -Wall) that enables multiple other flags implicitly. Reading through clang/include/clang/Basic/DiagnosticGroups.td in clang's sources, it looks like: 1. -Wformat is a group flag. 2. -Wformat-security is a member of the -Wformat group; enabling -Wformat will enable -Wformat-security. 3. -Wformat itself is a member of -Wmost (never heard of -Wmost, but w/e). So -Wmost will enable -Wformat will enable -Wformat-security. 4. -Wmost is itself a member of -Wall. -Wall enables -Wmost enables -Wformat enables -Wformat security. Looking now at Kbuild: 1. Makefile:523 adds -Wall to KBUILD_CFLAGS. 2. The same assignment expression but on line 526 immediately disables -Wformat-security via -Wno-format-security. 3. scripts/Makefile.extrawarn disables -Wformat via -Wno-format only for clang (via guard of CONFIG_CC_IS_CLANG). We _want_ -Wformat enabled for clang so that developers aren't sending patches that trigger -Wformat with GCC (if they didn't happen to test their code with both). It's disabled for clang until we can build the kernel cleanly with it enabled, which we'd like to do. I don't think that we need to enable -Wformat-security to build with -Wformat for clang. I suspect based on Randy's comment on patch 1/12 that perhaps -Wformat was _added_ to KBUILD_CFLAGS in scripts/Makefile.extrawarn rather than -Wno-format being _removed_. The former would re-enable -Wformat-security due to the grouping logic described above. The latter is probably closer to our ultimate goal of enabling -Wformat coverage for clang (or rather not disabling the coverage via -Wno-format; a double negative). I'm pretty sure the kernel doesn't support %n in format strings...see the comment above vsnprintf in lib/vsprintf.c. Are there other attacks other than %n that -Wformat-security guards against? Maybe there's some context on the commit that added -Wno-format-security to the kernel? Regardless, I don't think enabling -Wformat-security is a blocker for enabling -Wformat (or...disabling -Wno-format...two sides of the same coin) for clang. > > > > tldr: > > - printk(msg); > + printk("%s", msg); > > the only reason to make this change is where `msg' could contain a `%'. > Generally, it came from userspace. Otherwise these changes are a > useless consumer of runtime resources. > > I think it would be better to quieten clang in some fashion. -- Thanks, ~Nick Desaulniers