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=-8.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL 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 D03A4CA9EC5 for ; Wed, 30 Oct 2019 12:43:17 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 81E362080F for ; Wed, 30 Oct 2019 12:43:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="LOxVocb7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 81E362080F 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 1473E6B0007; Wed, 30 Oct 2019 08:43:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F8496B0008; Wed, 30 Oct 2019 08:43:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 00D696B000A; Wed, 30 Oct 2019 08:43:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0140.hostedemail.com [216.40.44.140]) by kanga.kvack.org (Postfix) with ESMTP id D408E6B0007 for ; Wed, 30 Oct 2019 08:43:16 -0400 (EDT) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id 679DB4820 for ; Wed, 30 Oct 2019 12:43:16 +0000 (UTC) X-FDA: 76100416392.13.straw66_399d82c6f325a X-HE-Tag: straw66_399d82c6f325a X-Filterd-Recvd-Size: 8690 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by imf37.hostedemail.com (Postfix) with ESMTP for ; Wed, 30 Oct 2019 12:43:15 +0000 (UTC) Received: by mail-wr1-f66.google.com with SMTP id p4so2133752wrm.8 for ; Wed, 30 Oct 2019 05:43:15 -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=nYHxVHtyXAKJumghycbs/2/ojEfG7xtsQvU8m0yFbDg=; b=LOxVocb7n/TY9vO8FzaoWzzlYv+99/o86gjYqbnE3WRAoxBQwO5gJb/nQ8zUC1TEeZ b0fGRXpLbdGQtp5s1hzfsWbTrpFxGZ6jgmfGppjy7gCFm7WhIBSBU7cHkRXRZcscqYOL zJhTIkNvzVDEacDMM5Mo4bc3KHiaL2dBbrc5/XbtlPk3ieP1eL/4o3vN5aDWM45dvnt6 Q0pEDzRkhY9qKTO3jY5fzBIoF5dSjyLMnHpAOdik/gr1m2J8ovE2JFgP1McYDFJ3l65S 4+0pbhHZ0fOfUK/HHOPreVcpaDk+HUBwqcyo0tmKXy74PBCgShNzP/LqgqAnQMo5ffgl rGJw== 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=nYHxVHtyXAKJumghycbs/2/ojEfG7xtsQvU8m0yFbDg=; b=dToHM2GxXr8wWYGl6ZxDgjQCjLexFAdJv+0++7OBWmbuo29msOfBE78OzduI1VewUk XBKvoqOHw7F6vNnKtQYEjay5EfXARYF5BuQESKl4FuAVjxQBeRUNlb1X+fUD/Md96po2 D+nKsBR6kpGvtaWldt/v7IGKJPL5CCExwRUhWcPENQ5In7JC/yP6Bu/MjLD/DklnWVoe U/yyz5a64bgLo8e/m83Ao8imLeBQMHzq52mES71AhWjyxD5V5AtSfgohT751ZZZZks4h An/pHobUDwS8prDxa+3TQ99HmQ5e0lg7xRPBJE3bT5N0Vu+j8ZQNRBktwbpTGSuhaikZ DT3w== X-Gm-Message-State: APjAAAUHSBzeUCkRUtqc+I3Ip6kWmo0Gq/Qhr+1yLBYT2XbXmd0UrlMk kHHc3/h6G5gCC7bVYqqqCdzsdPrLBq27FzDiikcV+Q== X-Google-Smtp-Source: APXvYqyboJztpwNGWvIk5VNLPtyw8wQ4Qg3Ng9OneLPf6DrkL57IoXMpE/SQBF9m88czYQFe8SaZauJrGTCapccVrqU= X-Received: by 2002:adf:8123:: with SMTP id 32mr23973204wrm.300.1572439393959; Wed, 30 Oct 2019 05:43:13 -0700 (PDT) MIME-Version: 1.0 References: <20191018094304.37056-1-glider@google.com> <20191018094304.37056-24-glider@google.com> <20191018162222.GM32665@bombadil.infradead.org> In-Reply-To: From: Alexander Potapenko Date: Wed, 30 Oct 2019 13:43:02 +0100 Message-ID: Subject: Re: [PATCH RFC v1 23/26] kmsan: unpoisoning buffers from devices etc. To: Matthew Wilcox Cc: Andrew Morton , Jens Axboe , "Theodore Ts'o" , Dmitry Torokhov , "Martin K . Petersen" , "Michael S. Tsirkin" , Christoph Hellwig , Eric Dumazet , Eric Van Hensbergen , Takashi Iwai , Vegard Nossum , 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 3:45 PM Alexander Potapenko wro= te: > > On Fri, Oct 18, 2019 at 6:22 PM Matthew Wilcox wrot= e: > > > > On Fri, Oct 18, 2019 at 11:43:01AM +0200, glider@google.com wrote: > > > When data is copied to memory from a device KMSAN should treat it as > > > initialized. In most cases it's enough to just unpoison the buffer th= at > > > is known to come from a device. > > > In the case with __do_page_cache_readahead() and bio_copy_user_iov() = we > > > have to mark the whole pages as ignored by KMSAN, as it's not obvious > > > where these pages are read again. > > > > ... > > > > > +++ b/mm/filemap.c > > > @@ -18,6 +18,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -2810,6 +2811,8 @@ static struct page *do_read_cache_page(struct a= ddress_space *mapping, > > > page =3D wait_on_page_read(page); > > > if (IS_ERR(page)) > > > return page; > > > + /* Assume all pages in page cache are initialized. */ > > > + kmsan_unpoison_shadow(page_address(page), PAGE_SIZE); > > > > Why would you do that? The page cache already keeps track of which > > pages are initialised -- the PageUptodate flag is set on them. Indeed, > > just adding a kmsan call to SetPageUptodate and __SetPageUptodate would > > probably be a very straightforward way of handling things, and probably > > means you can get rid of a lot of these other calls. > This seems like a very good thing to do, I'll definitely try that. Hm, turns out there's no need for this particular call to kmsan_unpoison_memory(), because the errors that it was suppressing are better addressed by making READ_ONCE_NOCHECK() return initialized memory. On the other hand, unpoisoning memory in SetPageUptodate doesn't seem to address the same errors. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D BUG: KMSAN: uninit-value in[< none >] get_wchan+0x2c6/0x410 arch/x86/kernel/process.c:851 CPU: 0 PID: 5159 Comm: ps Not tainted 5.4.0-rc5+ #3220 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01= /2014 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:77 [< none >] dump_stack+0x196/0x1f0 lib/dump_stack.c:113 [< none >] kmsan_report+0x127/0x220 mm/kmsan/kmsan_report.c:108 [< none >] __msan_warning+0x73/0xe0 mm/kmsan/kmsan_instr.c:245 [< none >] get_wchan+0x2c6/0x410 arch/x86/kernel/process.c:851 [< none >] do_task_stat+0x16f3/0x30a0 fs/proc/array.c:522 [< none >] proc_tgid_stat+0xbe/0xf0 fs/proc/array.c:632 [< none >] proc_single_show+0x1a8/0x2b0 fs/proc/base.c:756 [< none >] seq_read+0xad1/0x1da0 fs/seq_file.c:229 [< none >] __vfs_read+0x1a9/0xc90 fs/read_write.c:425 [< none >] vfs_read+0x333/0x6a0 fs/read_write.c:461 [< none >] ksys_read+0x281/0x460 fs/read_write.c:587 [< inline >] __do_sys_read fs/read_write.c:597 [< none >] __se_sys_read+0x92/0xb0 fs/read_write.c:595 [< none >] __x64_sys_read+0x4a/0x70 fs/read_write.c:595 [< none >] do_syscall_64+0xb6/0x160 arch/x86/entry/common.c:291 [< none >] entry_SYSCALL_64_after_hwframe+0x63/0xe7 arch/x86/entry/entry_64.S:177 Local variable description: ----stat.i@__se_sys_newstat Variable was created at: [< inline >] __do_sys_newstat fs/stat.c:337 [< none >] __se_sys_newstat+0x71/0xac0 fs/stat.c:337 [< inline >] __do_sys_newstat fs/stat.c:337 [< none >] __se_sys_newstat+0x71/0xac0 fs/stat.c:337 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > I however noticed that __SetPageUptodate is used when copying pages, > not during disk I/O. Is that really so? > > We basically need the following behavior: if a device writes to a > page, the contents of that page are considered initialized. > However when the kernel copies one page to another, we must explicitly > copy the source shadow page to the destination. > > On a related note, there seems to be a PG_dirty bit that indicates the > page is to be flushed to disk. > What's the best place to check such pages for being initialized, so > that we can also report writes of uninitialized data to the disk? > > > -- > 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 -- 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