On Tue, Dec 3, 2019 at 12:25 PM Marco Elver <elver@xxxxxxxxxx> wrote: > > On Tue, 3 Dec 2019 at 12:19, Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > > > On Mon, Dec 2, 2019 at 5:23 PM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > > > > > On Mon, Dec 2, 2019 at 4:51 PM Marco Elver <elver@xxxxxxxxxx> wrote: > > > > > > > > On Fri, 22 Nov 2019 at 12:28, <glider@xxxxxxxxxx> wrote: > > > > > > > > > > Disable the efficient 8-byte reading under KMSAN to avoid false positives. > > > > > > > > > > Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx> > > > > > To: Alexander Potapenko <glider@xxxxxxxxxx> > > > > > Cc: Vegard Nossum <vegard.nossum@xxxxxxxxxx> > > > > > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > > > > Cc: linux-mm@xxxxxxxxx > > > > > > > > > > --- > > > > > > > > > > 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. > > So was the previous version working because all strscpy where this was > relevant were unaligned? Right. > Re new patch: I think here it should be trivial to use 'if > (IS_ENABLED(CONFIG_KMSAN))' instead of #ifdef. Yes, sorry, I always forget about IS_ENABLED. > Thanks, > -- Marco > > > > > > +#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 -- 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
