Re: [PATCH RFC v3 31/36] kmsan: disable strscpy() optimization under KMSAN

From: Alexander Potapenko <glider@google.com>
To: Marco Elver <elver@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	 Linux Memory Management List <linux-mm@kvack.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	 Andreas Dilger <adilger.kernel@dilger.ca>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Andrey Konovalov <andreyknvl@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	 Andy Lutomirski <luto@kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	 Arnd Bergmann <arnd@arndb.de>,
	Christoph Hellwig <hch@infradead.org>,
	Christoph Hellwig <hch@lst.de>,
	 "Darrick J. Wong" <darrick.wong@oracle.com>,
	"David S. Miller" <davem@davemloft.net>,
	 Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Eric Biggers <ebiggers@google.com>,
	 Eric Dumazet <edumazet@google.com>,
	Eric Van Hensbergen <ericvh@gmail.com>,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Harry Wentland <harry.wentland@amd.com>,
	 Herbert Xu <herbert@gondor.apana.org.au>,
	Ilya Leoshkevich <iii@linux.ibm.com>,
	 Ingo Molnar <mingo@elte.hu>, Jason Wang <jasowang@redhat.com>,
	Jens Axboe <axboe@kernel.dk>,
	 Marek Szyprowski <m.szyprowski@samsung.com>,
	Mark Rutland <mark.rutland@arm.com>,
	 "Martin K. Petersen" <martin.petersen@oracle.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	 Matthew Wilcox <willy@infradead.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Michal Simek <monstr@monstr.eu>,  Petr Mladek <pmladek@suse.com>,
	Qian Cai <cai@lca.pw>, Randy Dunlap <rdunlap@infradead.org>,
	 Robin Murphy <robin.murphy@arm.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	 Steven Rostedt <rostedt@goodmis.org>,
	Takashi Iwai <tiwai@suse.com>, "Theodore Ts'o" <tytso@mit.edu>,
	 Thomas Gleixner <tglx@linutronix.de>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Wolfram Sang <wsa@the-dreams.de>
Subject: Re: [PATCH RFC v3 31/36] kmsan: disable strscpy() optimization under KMSAN
Date: Tue, 3 Dec 2019 12:19:16 +0100	[thread overview]
Message-ID: <CAG_fn=Xgv5oQY65RiCbw2cN7gNt41n1HbJUFjwD3N5nuP-yxdQ@mail.gmail.com> (raw)
In-Reply-To: <CAG_fn=WgeCY0d2tPmfCL9G7T=KJVEH5WmTp0t_GVq7Z8STnb0Q@mail.gmail.com>

On Mon, Dec 2, 2019 at 5:23 PM Alexander Potapenko <glider@google.com> wrote:
>
> On Mon, Dec 2, 2019 at 4:51 PM Marco Elver <elver@google.com> wrote:
> >
> > On Fri, 22 Nov 2019 at 12:28, <glider@google.com> wrote:
> > >
> > > Disable the efficient 8-byte reading under KMSAN to avoid false positives.
> > >
> > > Signed-off-by: Alexander Potapenko <glider@google.com>
> > > To: Alexander Potapenko <glider@google.com>
> > > Cc: Vegard Nossum <vegard.nossum@oracle.com>
> > > Cc: Dmitry Vyukov <dvyukov@google.com>
> > > Cc: linux-mm@kvack.org
> > >
> > > ---
> > >
> > > Change-Id: I25d1acf5c3df6eff85894cd94f5ddbe93308271c
> > > ---
> > >  lib/string.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/string.c b/lib/string.c
> > > index 08ec58cc673b..15efdc51bda6 100644
> > > --- a/lib/string.c
> > > +++ b/lib/string.c
> > > @@ -186,7 +186,10 @@ ssize_t strscpy(char *dest, const char *src, size_t count)
> > >         if (count == 0 || WARN_ON_ONCE(count > INT_MAX))
> > >                 return -E2BIG;
> > >
> > > -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> > > +/**
> >
> > Why a doc comment?
> Will fix, thanks!
> > > + * Disable the efficient 8-byte reading under KMSAN to avoid false positives.
> > > + */
> >
> > AFAIK the CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS case is about
> > unaligned accesses crossing page boundaries. In the #else case it's
> > still going to do word-at-a-time if both src and dest are aligned, so
> > the comment above is somewhat inaccurate.
> Yes, this makes little sense.
> Reading word-at-a-time shouldn't induce any errors, although it may
> generate redundant stack IDs for values that will never be used.
> I'll try to drop this patch.
Turns out the patch is still needed, as read_word_at_a_time may read
uninitialized bytes which are then used in comparisons.
I've changed the patch to always set max=0 under KMSAN:
https://github.com/google/kmsan/commit/3ff43863bf53dd871a3d4dc4fbb2a76d79b4db4f
Will include this version in v4 series.
>
> > > +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && !defined(CONFIG_KMSAN)
> > >         /*
> > >          * If src is unaligned, don't cross a page boundary,
> > >          * since we don't know if the next page is mapped.
> > > --
> > > 2.24.0.432.g9d3f5f5b63-goog
> > >
>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: 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ße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg