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=-13.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=ham 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 4D1B2CA9EAE for ; Tue, 29 Oct 2019 12:45:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0257D2086A for ; Tue, 29 Oct 2019 12:45:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="be8LepT5" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0257D2086A Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 974C36B0006; Tue, 29 Oct 2019 08:45:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9269A6B0007; Tue, 29 Oct 2019 08:45:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 83B076B0008; Tue, 29 Oct 2019 08:45:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0086.hostedemail.com [216.40.44.86]) by kanga.kvack.org (Postfix) with ESMTP id 616266B0006 for ; Tue, 29 Oct 2019 08:45:21 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with SMTP id 1D032180AD838 for ; Tue, 29 Oct 2019 12:45:21 +0000 (UTC) X-FDA: 76096792842.29.shirt08_1189a20458520 X-HE-Tag: shirt08_1189a20458520 X-Filterd-Recvd-Size: 7653 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by imf17.hostedemail.com (Postfix) with ESMTP for ; Tue, 29 Oct 2019 12:45:20 +0000 (UTC) Received: by mail-wr1-f65.google.com with SMTP id n15so13455499wrw.13 for ; Tue, 29 Oct 2019 05:45:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=PYM1JyZApJJ9p3uH/neUYlUnpnJqzgsuQRRDhmwoP7U=; b=be8LepT5UHLT4k8Mwc4kLd7UHkRzqSR0kt6zNWAcW3buluXyV3D6kdwaZwg/bZgvSd 1YVfBFRl0kg5NiOw1ga8l6zwpmgX3Iq/2CrELgvHPl3j+W4ZK35CgIN1jXVYC8Jjzhg2 SqeejblRBn97d3bMGgcr53pPXpKDHWjJmsxskuu6OvVcAbUxQzAZs2jS/gVxYyY94Rr6 AQz86lvz34bM5WQ0993NVsrQrK1E7dpAZgRa2pKGl1Iu+HmaRxm3MhI4WEmgiQnKZoRe 3xkwQIbXwmgkr2BZfsBaqByv9vQNHZ+kkHgpeJnHfqiaSPz3G2Bu0JXannmTn/sPqTaf cqWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=PYM1JyZApJJ9p3uH/neUYlUnpnJqzgsuQRRDhmwoP7U=; b=EXxhaXvTcn1NAmMfUhJk0CYd0Owux20fQnNbx0xryT5ft7ycR9/0hA77gFonsxzIaB 5H3175slayq6tze4z3y4S9QWQ0ek1tIcc+ui5IGstvhaZkB6HsJ2d7nDYuP3V1naHVoz NRRg8eTeALCgLB8rnzs1n64YuFVWvzY/reOA4IcYdM5ZSylX1ENbyP1dBmInsNDDin16 eNRQRjztjAy634p26qKg/fxOrdiJ1KAn67lo3IpFVCLIdmIY0eP9NoPZ7pzg6JD5Q8Ul VY2SxyYoFNqkNSIwlWajbDNfsFbFsCNUYy8kwC9ClzmpP6jrVL6uyVx/+t9ryEicNd78 FGqg== X-Gm-Message-State: APjAAAWIpt50dBVOWDJNhpfT0SoPeyT3ywn58araMpg5dOlXtztLnj34 Xi6oCUHIY+fwJ+VCJFzqEE9zSYHC5pPV12xFLF4qsw== X-Google-Smtp-Source: APXvYqxX/+02YAgK8CPjjwf8CRV2hmVR4Fg+IGNkE47/T8lKL6RdjW7huRwhWAqT/PUv3odrUOpDl4iiDJU8KtOWrV4= X-Received: by 2002:adf:9ec7:: with SMTP id b7mr19908466wrf.221.1572353118521; Tue, 29 Oct 2019 05:45:18 -0700 (PDT) MIME-Version: 1.0 References: <20191018094304.37056-1-glider@google.com> <20191018094304.37056-6-glider@google.com> <20191021090925.3dcqukovauqsyw5w@pathway.suse.cz> <20191024124634.hzsn74l7q4nl4mti@pathway.suse.cz> <20191029120235.uncxoraf4ex2haut@pathway.suse.cz> In-Reply-To: <20191029120235.uncxoraf4ex2haut@pathway.suse.cz> From: Alexander Potapenko Date: Tue, 29 Oct 2019 13:45:06 +0100 Message-ID: Subject: Re: [PATCH RFC v1 05/26] printk_safe: externalize printk_context To: Petr Mladek Cc: Vegard Nossum , Sergey Senozhatsky , Steven Rostedt , Dmitry Vyukov , Linux Memory Management List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Tue, Oct 29, 2019 at 1:02 PM Petr Mladek wrote: > > On Mon 2019-10-28 14:09:29, Alexander Potapenko wrote: > > On Thu, Oct 24, 2019 at 2:46 PM Petr Mladek wrote: > > > On Wed 2019-10-23 19:57:39, Alexander Potapenko wrote: > > > > What I'm seeing now is that e.g. in the following case: > > > > ptr =3D kmalloc(sizeof(int), GFP_KERNEL); > > > > if (ptr) > > > > pr_info("true\n"); > > > > else > > > > pr_info("false\n"); > > > > > > > > KMSAN detects errors within pr_info(), namely in vprintk_store(). > > > > If I understand correctly, printing from that point causes printk t= o > > > > use the per-cpu buffer, which is flushed once we're done with the > > > > original printing: > > > > > > > > [ 58.984971][ T8390] BUG: KMSAN: uninit-value in > > > > kmsan_handle_vprintk+0xa0/0xf0 > > > > ... > > > > [ 59.061976][ C0] BUG: KMSAN: uninit-value in vsnprintf+0x3276= /0x32b0 > > > > ... > > > > [ 59.062457][ C0] BUG: KMSAN: uninit-value in format_decode+0x= 17f/0x1900 > > > > ... > > > > [ 59.062961][ C0] BUG: KMSAN: uninit-value in format_decode+0x= 17f/0x1900 > > > > > > Please, do you have an explanation where the uninitialized values com= e > > > from? Is it a false positive? Or is there really a bug how the > > > printk() parameters are passed? > > I believe these are true bugs. > > The problem is, when we pass an uninitialized argument into printk(), > > KMSAN will report an error every time this uninitialized argument is > > used. > > I see, thanks for explanation. > > > E.g. for an uninitialized format string there'll be at least > > strlen(fmt) reports in format_decode(), followed by several reports in > > vsnprintf(). > > Although these reports seem to be real, printing only the first of > > them should be more than enough. > > Isn't this a generic problem? I mean that uninitialized values > can be passed and used also in many other locations. More or less, yes. > printk() is special because this problem causes infinite loop. But it > would make sense to report also any other problematic value only once. Yes, printk is a bit different, because finding errors in it causes other printk() calls which now don't cause infinite loops, but may produce out-of-order error messages. > > > In the future we'll actually want KMSAN to check every printk() > > argument (which will require parsing the format string to figure out > > the arguments' lengths), but disable reports within printk. > > What is the motivation for this, please? > > It looks to me that you want to do very paranoid checks of variables > passed to printk()? Do you want to prevent printk() crashes? Or > do you want to make sure that printk() produces correct values? Simply passing an uninitialized value to printk() may result in cryptic error messages from printk guts, which may be harder to understand without knowing how printk works. The idea is to check the arguments right before printk, so that we can print a really simple diagnostic message. > From my POV, printk() is debugging tool. It is used to show values > that people are interested in. On one hand, it might make sense to warn > people that a particular value was not initalized. On the other hand, > printk() is not important for the kernel behavior. It just > reads values and does not affect any behavior. > > I would like to understand how many printk-related code is > worth the effort. I think overall you're right. The feature I'm talking about isn't a critical part of KMSAN functionality, so in order to keep the code simple I'd better drop it, as long as we're able to report bugs when uninitialized memory is passed to printk. The only change to printk that I'll probably still have to make is to initialize the result of vscnprintf() here: https://github.com/google/kmsan/blob/master/kernel/printk/printk.c#L1921 Passing uninitialized data to vscnprintf() causes its result to also be uninitialized, which causes an avalanche of new reports in vprintk_store() I'll send the updated patch (together with other KMSAN patches) your way to= day. > Best Regards, > Petr --=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, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg