linux-kernel - Re: [PATCH] lkdtm: fix maybe-uninitialized warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jJ+c2-x72P95DUcb-V6SKmhpbayW2gv=zu2vpF_R5ScVQ@mail.gmail.com>
Date:	Tue, 26 Jul 2016 08:21:37 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] lkdtm: fix maybe-uninitialized warning

On Tue, Jul 26, 2016 at 5:28 AM, Arnd Bergmann <arnd@...db.de> wrote:
> The do_usercopy_stack() function uses uninitialized stack data to initialize
> more of the stack, which causes a warning in some configurations (ARM allmodconfig):
>
> drivers/misc/lkdtm_usercopy.c:52:15: warning: 'bad_stack' may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> The warning gets reports by Mark Brown's build bot and looks correct (we are trying
> to trick the compiler here, and sometimes the compiler notices), and I could reproduce
> it with gcc-4.7 through gcc-5.3 but not gcc-6.1 for some reason.
>
> This changes the code to use the low byte of the address of the stack to initialize
> the stack data, instead of using data from the stack itself, to avoid the warning.
>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> Fixes: a3dff71c1c88 ("lkdtm: split usercopy tests to separate file")

Acked-by: Kees Cook <keescook@...omium.org>

I thought I already sent this fix to Greg? Maybe it got lost...

-Kees

> ---
>  drivers/misc/lkdtm_usercopy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/lkdtm_usercopy.c b/drivers/misc/lkdtm_usercopy.c
> index 5a3fd76eec27..5525a204db93 100644
> --- a/drivers/misc/lkdtm_usercopy.c
> +++ b/drivers/misc/lkdtm_usercopy.c
> @@ -49,7 +49,7 @@ static noinline void do_usercopy_stack(bool to_user, bool bad_frame)
>
>         /* This is a pointer to outside our current stack frame. */
>         if (bad_frame) {
> -               bad_stack = do_usercopy_stack_callee((uintptr_t)bad_stack);
> +               bad_stack = do_usercopy_stack_callee((uintptr_t)&bad_stack);
>         } else {
>                 /* Put start address just inside stack. */
>                 bad_stack = task_stack_page(current) + THREAD_SIZE;
> --
> 2.9.0
>



-- 
Kees Cook
Chrome OS & Brillo Security