linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [v2] kasan: avoid -Wmaybe-uninitialized warning
@ 2017-07-21 21:02 Arnd Bergmann
  2017-07-24 11:35 ` Alexander Potapenko
  0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2017-07-21 21:02 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Arnd Bergmann, Alexander Potapenko, Dmitry Vyukov, Andrew Morton,
	Andrey Konovalov, kasan-dev, linux-mm, linux-kernel

gcc-7 produces this warning:

mm/kasan/report.c: In function 'kasan_report':
mm/kasan/report.c:351:3: error: 'info.first_bad_addr' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   print_shadow_for_address(info->first_bad_addr);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/report.c:360:27: note: 'info.first_bad_addr' was declared here

The code seems fine as we only print info.first_bad_addr when there is a shadow,
and we always initialize it in that case, but this is relatively hard
for gcc to figure out after the latest rework. Adding an intialization
in the other code path gets rid of the warning.

Fixes: b235b9808664 ("kasan: unify report headers")
Link: https://patchwork.kernel.org/patch/9641417/
Acked-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Originally submitted on March 23, but unfortunately is still needed,
as verified on 4.13-rc1, with aarch64-linux-gcc-7.1.1

v2: add a comment as Andrew suggested
---
 mm/kasan/report.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 04bb1d3eb9ec..28fb222ab149 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -111,6 +111,9 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
 {
 	const char *bug_type = "unknown-crash";
 
+	/* shut up spurious -Wmaybe-uninitialized warning */
+	info->first_bad_addr = (void *)(-1ul);
+
 	if ((unsigned long)info->access_addr < PAGE_SIZE)
 		bug_type = "null-ptr-deref";
 	else if ((unsigned long)info->access_addr < TASK_SIZE)
-- 
2.9.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] [v2] kasan: avoid -Wmaybe-uninitialized warning
  2017-07-21 21:02 [PATCH] [v2] kasan: avoid -Wmaybe-uninitialized warning Arnd Bergmann
@ 2017-07-24 11:35 ` Alexander Potapenko
  2017-07-25  7:17   ` Arnd Bergmann
  0 siblings, 1 reply; 5+ messages in thread
From: Alexander Potapenko @ 2017-07-24 11:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrey Ryabinin, Dmitry Vyukov, Andrew Morton, Andrey Konovalov,
	kasan-dev, Linux Memory Management List, LKML

On Fri, Jul 21, 2017 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> gcc-7 produces this warning:
>
> mm/kasan/report.c: In function 'kasan_report':
> mm/kasan/report.c:351:3: error: 'info.first_bad_addr' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    print_shadow_for_address(info->first_bad_addr);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/report.c:360:27: note: 'info.first_bad_addr' was declared here
>
> The code seems fine as we only print info.first_bad_addr when there is a shadow,
> and we always initialize it in that case, but this is relatively hard
> for gcc to figure out after the latest rework. Adding an intialization
> in the other code path gets rid of the warning.
>
> Fixes: b235b9808664 ("kasan: unify report headers")
> Link: https://patchwork.kernel.org/patch/9641417/
> Acked-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Originally submitted on March 23, but unfortunately is still needed,
> as verified on 4.13-rc1, with aarch64-linux-gcc-7.1.1
>
> v2: add a comment as Andrew suggested
> ---
>  mm/kasan/report.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 04bb1d3eb9ec..28fb222ab149 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -111,6 +111,9 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
>  {
>         const char *bug_type = "unknown-crash";
>
> +       /* shut up spurious -Wmaybe-uninitialized warning */
> +       info->first_bad_addr = (void *)(-1ul);
> +
Why don't we initialize info.first_bad_addr in kasan_report(), where
info is allocated?
>         if ((unsigned long)info->access_addr < PAGE_SIZE)
>                 bug_type = "null-ptr-deref";
>         else if ((unsigned long)info->access_addr < TASK_SIZE)
> --
> 2.9.0
>



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [v2] kasan: avoid -Wmaybe-uninitialized warning
  2017-07-24 11:35 ` Alexander Potapenko
@ 2017-07-25  7:17   ` Arnd Bergmann
  2017-07-25 12:06     ` Andrey Ryabinin
  0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2017-07-25  7:17 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Andrey Ryabinin, Dmitry Vyukov, Andrew Morton, Andrey Konovalov,
	kasan-dev, Linux Memory Management List, LKML

On Mon, Jul 24, 2017 at 1:35 PM, Alexander Potapenko <glider@google.com> wrote:
> On Fri, Jul 21, 2017 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:

>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>> index 04bb1d3eb9ec..28fb222ab149 100644
>> --- a/mm/kasan/report.c
>> +++ b/mm/kasan/report.c
>> @@ -111,6 +111,9 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
>>  {
>>         const char *bug_type = "unknown-crash";
>>
>> +       /* shut up spurious -Wmaybe-uninitialized warning */
>> +       info->first_bad_addr = (void *)(-1ul);
>> +
> Why don't we initialize info.first_bad_addr in kasan_report(), where
> info is allocated?

I'm just trying to shut up a particular warning here where gcc can't figure out
by itself that it is initialized. Setting an invalid address at
allocation time would
prevent gcc from warning even for any trivial bug where we use the incorrect
value in the normal code path, in case someone later wants to modify the
code further and makes a mistake.

       Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [v2] kasan: avoid -Wmaybe-uninitialized warning
  2017-07-25  7:17   ` Arnd Bergmann
@ 2017-07-25 12:06     ` Andrey Ryabinin
  2017-07-25 14:51       ` Arnd Bergmann
  0 siblings, 1 reply; 5+ messages in thread
From: Andrey Ryabinin @ 2017-07-25 12:06 UTC (permalink / raw)
  To: Arnd Bergmann, Alexander Potapenko
  Cc: Dmitry Vyukov, Andrew Morton, Andrey Konovalov, kasan-dev,
	Linux Memory Management List, LKML

On 07/25/2017 10:17 AM, Arnd Bergmann wrote:
> On Mon, Jul 24, 2017 at 1:35 PM, Alexander Potapenko <glider@google.com> wrote:
>> On Fri, Jul 21, 2017 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
>>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>>> index 04bb1d3eb9ec..28fb222ab149 100644
>>> --- a/mm/kasan/report.c
>>> +++ b/mm/kasan/report.c
>>> @@ -111,6 +111,9 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
>>>  {
>>>         const char *bug_type = "unknown-crash";
>>>
>>> +       /* shut up spurious -Wmaybe-uninitialized warning */
>>> +       info->first_bad_addr = (void *)(-1ul);
>>> +
>> Why don't we initialize info.first_bad_addr in kasan_report(), where
>> info is allocated?
> 
> I'm just trying to shut up a particular warning here where gcc can't figure out
> by itself that it is initialized. Setting an invalid address at
> allocation time would
> prevent gcc from warning even for any trivial bug where we use the incorrect
> value in the normal code path, in case someone later wants to modify the
> code further and makes a mistake.
> 

'info->first_bad_addr' could be initialized to the correct value. That would be 'addr' itself
for 'wild' type of bugs.
Initialization in get_wild_bug_type() looks a bit odd and off-place.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [v2] kasan: avoid -Wmaybe-uninitialized warning
  2017-07-25 12:06     ` Andrey Ryabinin
@ 2017-07-25 14:51       ` Arnd Bergmann
  0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2017-07-25 14:51 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Alexander Potapenko, Dmitry Vyukov, Andrew Morton,
	Andrey Konovalov, kasan-dev, Linux Memory Management List, LKML

On Tue, Jul 25, 2017 at 2:06 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> On 07/25/2017 10:17 AM, Arnd Bergmann wrote:
>> On Mon, Jul 24, 2017 at 1:35 PM, Alexander Potapenko <glider@google.com> wrote:
>>> On Fri, Jul 21, 2017 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>>>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>>>> index 04bb1d3eb9ec..28fb222ab149 100644
>>>> --- a/mm/kasan/report.c
>>>> +++ b/mm/kasan/report.c
>>>> @@ -111,6 +111,9 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
>>>>  {
>>>>         const char *bug_type = "unknown-crash";
>>>>
>>>> +       /* shut up spurious -Wmaybe-uninitialized warning */
>>>> +       info->first_bad_addr = (void *)(-1ul);
>>>> +
>>> Why don't we initialize info.first_bad_addr in kasan_report(), where
>>> info is allocated?
>>
>> I'm just trying to shut up a particular warning here where gcc can't figure out
>> by itself that it is initialized. Setting an invalid address at
>> allocation time would
>> prevent gcc from warning even for any trivial bug where we use the incorrect
>> value in the normal code path, in case someone later wants to modify the
>> code further and makes a mistake.
>>
>
> 'info->first_bad_addr' could be initialized to the correct value. That would be 'addr' itself
> for 'wild' type of bugs.
> Initialization in get_wild_bug_type() looks a bit odd and off-place.

Yes, that makes sense. I'll send a new version then.

        Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-07-25 14:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-21 21:02 [PATCH] [v2] kasan: avoid -Wmaybe-uninitialized warning Arnd Bergmann
2017-07-24 11:35 ` Alexander Potapenko
2017-07-25  7:17   ` Arnd Bergmann
2017-07-25 12:06     ` Andrey Ryabinin
2017-07-25 14:51       ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).