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

[Kees Cook]

On Tue, Jul 26, 2016 at 5:28 AM, Arnd Bergmann <arnd@xxxxxxxx> 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@xxxxxxxx>
> Fixes: a3dff71c1c88 ("lkdtm: split usercopy tests to separate file")

Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

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