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

[Arnd Bergmann]

On Tuesday, July 26, 2016 8:21:37 AM CEST Kees Cook wrote:
> On Tue, Jul 26, 2016 at 5:28 AM, Arnd Bergmann  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 
> > Fixes: a3dff71c1c88 ("lkdtm: split usercopy tests to separate file")
> 
> Acked-by: Kees Cook 
> 
> I thought I already sent this fix to Greg?

Possible. I mentioned the problem to you when it first showed up,
but noticed today that I didn't have a patch for it in my testing
tree (since I test with gcc-6.1, which doesn't show the bug).

> Maybe it got lost...

More likely that it's still in his backlog then.

Arnd

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

[Arnd Bergmann]

On Tuesday, July 26, 2016 8:21:37 AM CEST Kees Cook wrote:
> On Tue, Jul 26, 2016 at 5:28 AM, Arnd Bergmann  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 
> > Fixes: a3dff71c1c88 ("lkdtm: split usercopy tests to separate file")
> 
> Acked-by: Kees Cook 
> 
> I thought I already sent this fix to Greg?

Possible. I mentioned the problem to you when it first showed up,
but noticed today that I didn't have a patch for it in my testing
tree (since I test with gcc-6.1, which doesn't show the bug).

> Maybe it got lost...

More likely that it's still in his backlog then.

Arnd

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

[Kees Cook]

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

Acked-by: Kees Cook 

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)_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

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

[Kees Cook]

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

Acked-by: Kees Cook 

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)_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

[PATCH] lkdtm: fix maybe-uninitialized warning

[Arnd Bergmann]

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 
Fixes: a3dff71c1c88 ("lkdtm: split usercopy tests to separate file")
---
 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)_stack);
} else {
/* Put start address just inside stack. */
bad_stack = task_stack_page(current) + THREAD_SIZE;
-- 
2.9.0

[PATCH] lkdtm: fix maybe-uninitialized warning

[Arnd Bergmann]

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 
Fixes: a3dff71c1c88 ("lkdtm: split usercopy tests to separate file")
---
 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)_stack);
} else {
/* Put start address just inside stack. */
bad_stack = task_stack_page(current) + THREAD_SIZE;
-- 
2.9.0