All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Axtens <dja@axtens.net>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: kbuild test robot <lkp@intel.com>,
	kbuild-all@lists.01.org, Johannes Weiner <hannes@cmpxchg.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
Date: Fri, 05 Jun 2020 20:35:21 +1000	[thread overview]
Message-ID: <87img5u7fa.fsf@dja-thinkpad.axtens.net> (raw)
In-Reply-To: <20200602121620.257a40752a9ce475d8a2c6c8@linux-foundation.org>

Andrew Morton <akpm@linux-foundation.org> writes:

> On Tue, 02 Jun 2020 15:55:27 +1000 Daniel Axtens <dja@axtens.net> wrote:
>
>> Huh, turns out that we do actually see a similar set of warnings before
>> and after this patch, they're just different warnings as a consequence
>> of my patch.
>
> Ah.
>  
>> Given that these warnings only show up at W=1, is there any point in
>> further supressing them?
>
> I guess not.
>
> So what should we do?  Go over the various sites and use memcpy(), each
> with a suitably apologetic code comment explaining the reason?

It's a good question.

 - We could disable the warning outright or demote it to W=2+. It
   doesn't tell us about behaviour that is definitely wrong, just
   suspicious, and we have a number of callsites in the kenel using it
   in a correct and compliant way. Wstringop-overflow catches
   full-on overflows (once Linus reenables it).

 - We could keep the warning and patch the callsites. One can make the
   argument that changing to memcpy makes the semantics more clear: str*
   in C refers to null-terminated strings, and if the string isn't
   null-terminated then the str* functions aren't a natural fit for
   manipulating it. And it means we get to keep the warning enabled to
   catch accidental off-by-ones.

 - We could do nothing. If you compile with W=1 there is a known set of
   warnings and you're probably more interested in the delta than in the
   set.

I think I have a slight preference for patching the callsites and
eventually promoting the warning from W=1 to always on. But I have to be
honest that I'm not going to have time to do it soon, and potentially
not for a few months.

Regards,
Daniel


WARNING: multiple messages have this Message-ID (diff)
From: Daniel Axtens <dja@axtens.net>
To: kbuild-all@lists.01.org
Subject: Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
Date: Fri, 05 Jun 2020 20:35:21 +1000	[thread overview]
Message-ID: <87img5u7fa.fsf@dja-thinkpad.axtens.net> (raw)
In-Reply-To: <20200602121620.257a40752a9ce475d8a2c6c8@linux-foundation.org>

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

Andrew Morton <akpm@linux-foundation.org> writes:

> On Tue, 02 Jun 2020 15:55:27 +1000 Daniel Axtens <dja@axtens.net> wrote:
>
>> Huh, turns out that we do actually see a similar set of warnings before
>> and after this patch, they're just different warnings as a consequence
>> of my patch.
>
> Ah.
>  
>> Given that these warnings only show up at W=1, is there any point in
>> further supressing them?
>
> I guess not.
>
> So what should we do?  Go over the various sites and use memcpy(), each
> with a suitably apologetic code comment explaining the reason?

It's a good question.

 - We could disable the warning outright or demote it to W=2+. It
   doesn't tell us about behaviour that is definitely wrong, just
   suspicious, and we have a number of callsites in the kenel using it
   in a correct and compliant way. Wstringop-overflow catches
   full-on overflows (once Linus reenables it).

 - We could keep the warning and patch the callsites. One can make the
   argument that changing to memcpy makes the semantics more clear: str*
   in C refers to null-terminated strings, and if the string isn't
   null-terminated then the str* functions aren't a natural fit for
   manipulating it. And it means we get to keep the warning enabled to
   catch accidental off-by-ones.

 - We could do nothing. If you compile with W=1 there is a known set of
   warnings and you're probably more interested in the delta than in the
   set.

I think I have a slight preference for patching the callsites and
eventually promoting the warning from W=1 to always on. But I have to be
honest that I'm not going to have time to do it soon, and potentially
not for a few months.

Regards,
Daniel

  reply	other threads:[~2020-06-05 10:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-30 10:07 [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy' kbuild test robot
2020-05-30 10:07 ` kbuild test robot
2020-06-02  1:04 ` Andrew Morton
2020-06-02  1:04   ` Andrew Morton
2020-06-02  3:26   ` Daniel Axtens
2020-06-02  3:26     ` Daniel Axtens
2020-06-02  3:53     ` Daniel Axtens
2020-06-02  3:53       ` Daniel Axtens
2020-06-02  5:55       ` Daniel Axtens
2020-06-02  5:55         ` Daniel Axtens
2020-06-02 19:16         ` Andrew Morton
2020-06-02 19:16           ` Andrew Morton
2020-06-05 10:35           ` Daniel Axtens [this message]
2020-06-05 10:35             ` Daniel Axtens

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87img5u7fa.fsf@dja-thinkpad.axtens.net \
    --to=dja@axtens.net \
    --cc=akpm@linux-foundation.org \
    --cc=dvyukov@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.