linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose
@ 2020-01-02 23:49 Alexander Popov
  2020-01-14 12:14 ` Alexander Popov
  2020-01-22 11:58 ` Alexander Popov
  0 siblings, 2 replies; 6+ messages in thread
From: Alexander Popov @ 2020-01-02 23:49 UTC (permalink / raw)
  To: Kees Cook, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	Alexander Popov
  Cc: notify

Make the stack erasing test more verbose about the errors that it
can detect.

Signed-off-by: Alexander Popov <alex.popov@linux.com>
---
 drivers/misc/lkdtm/stackleak.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/lkdtm/stackleak.c b/drivers/misc/lkdtm/stackleak.c
index d5a084475abc..d1a5c0705be3 100644
--- a/drivers/misc/lkdtm/stackleak.c
+++ b/drivers/misc/lkdtm/stackleak.c
@@ -16,6 +16,7 @@ void lkdtm_STACKLEAK_ERASING(void)
 	unsigned long *sp, left, found, i;
 	const unsigned long check_depth =
 			STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long);
+	bool test_failed = false;
 
 	/*
 	 * For the details about the alignment of the poison values, see
@@ -34,7 +35,8 @@ void lkdtm_STACKLEAK_ERASING(void)
 		left--;
 	} else {
 		pr_err("FAIL: not enough stack space for the test\n");
-		return;
+		test_failed = true;
+		goto end;
 	}
 
 	pr_info("checking unused part of the thread stack (%lu bytes)...\n",
@@ -52,22 +54,29 @@ void lkdtm_STACKLEAK_ERASING(void)
 	}
 
 	if (found <= check_depth) {
-		pr_err("FAIL: thread stack is not erased (checked %lu bytes)\n",
+		pr_err("FAIL: the erased part is not found (checked %lu bytes)\n",
 						i * sizeof(unsigned long));
-		return;
+		test_failed = true;
+		goto end;
 	}
 
-	pr_info("first %lu bytes are unpoisoned\n",
+	pr_info("the erased part begins after %lu not poisoned bytes\n",
 				(i - found) * sizeof(unsigned long));
 
 	/* The rest of thread stack should be erased */
 	for (; i < left; i++) {
 		if (*(sp - i) != STACKLEAK_POISON) {
-			pr_err("FAIL: thread stack is NOT properly erased\n");
-			return;
+			pr_err("FAIL: bad value number %lu in the erased part: 0x%lx\n",
+								i, *(sp - i));
+			test_failed = true;
 		}
 	}
 
-	pr_info("OK: the rest of the thread stack is properly erased\n");
-	return;
+end:
+	if (test_failed) {
+		pr_err("FAIL: the thread stack is NOT properly erased\n");
+		dump_stack();
+	} else {
+		pr_info("OK: the rest of the thread stack is properly erased\n");
+	}
 }
-- 
2.23.0


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

* Re: [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose
  2020-01-02 23:49 [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose Alexander Popov
@ 2020-01-14 12:14 ` Alexander Popov
  2020-01-22 11:58 ` Alexander Popov
  1 sibling, 0 replies; 6+ messages in thread
From: Alexander Popov @ 2020-01-14 12:14 UTC (permalink / raw)
  To: Kees Cook, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel; +Cc: notify

Hello!

Let me remind of this patch.

Best regards,
Alexander


On 03.01.2020 02:49, Alexander Popov wrote:
> Make the stack erasing test more verbose about the errors that it
> can detect.
> 
> Signed-off-by: Alexander Popov <alex.popov@linux.com>
> ---
>  drivers/misc/lkdtm/stackleak.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/misc/lkdtm/stackleak.c b/drivers/misc/lkdtm/stackleak.c
> index d5a084475abc..d1a5c0705be3 100644
> --- a/drivers/misc/lkdtm/stackleak.c
> +++ b/drivers/misc/lkdtm/stackleak.c
> @@ -16,6 +16,7 @@ void lkdtm_STACKLEAK_ERASING(void)
>  	unsigned long *sp, left, found, i;
>  	const unsigned long check_depth =
>  			STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long);
> +	bool test_failed = false;
>  
>  	/*
>  	 * For the details about the alignment of the poison values, see
> @@ -34,7 +35,8 @@ void lkdtm_STACKLEAK_ERASING(void)
>  		left--;
>  	} else {
>  		pr_err("FAIL: not enough stack space for the test\n");
> -		return;
> +		test_failed = true;
> +		goto end;
>  	}
>  
>  	pr_info("checking unused part of the thread stack (%lu bytes)...\n",
> @@ -52,22 +54,29 @@ void lkdtm_STACKLEAK_ERASING(void)
>  	}
>  
>  	if (found <= check_depth) {
> -		pr_err("FAIL: thread stack is not erased (checked %lu bytes)\n",
> +		pr_err("FAIL: the erased part is not found (checked %lu bytes)\n",
>  						i * sizeof(unsigned long));
> -		return;
> +		test_failed = true;
> +		goto end;
>  	}
>  
> -	pr_info("first %lu bytes are unpoisoned\n",
> +	pr_info("the erased part begins after %lu not poisoned bytes\n",
>  				(i - found) * sizeof(unsigned long));
>  
>  	/* The rest of thread stack should be erased */
>  	for (; i < left; i++) {
>  		if (*(sp - i) != STACKLEAK_POISON) {
> -			pr_err("FAIL: thread stack is NOT properly erased\n");
> -			return;
> +			pr_err("FAIL: bad value number %lu in the erased part: 0x%lx\n",
> +								i, *(sp - i));
> +			test_failed = true;
>  		}
>  	}
>  
> -	pr_info("OK: the rest of the thread stack is properly erased\n");
> -	return;
> +end:
> +	if (test_failed) {
> +		pr_err("FAIL: the thread stack is NOT properly erased\n");
> +		dump_stack();
> +	} else {
> +		pr_info("OK: the rest of the thread stack is properly erased\n");
> +	}
>  }
> 


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

* Re: [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose
  2020-01-02 23:49 [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose Alexander Popov
  2020-01-14 12:14 ` Alexander Popov
@ 2020-01-22 11:58 ` Alexander Popov
  2020-01-27 23:15   ` Kees Cook
  1 sibling, 1 reply; 6+ messages in thread
From: Alexander Popov @ 2020-01-22 11:58 UTC (permalink / raw)
  To: Kees Cook, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel; +Cc: notify

On 03.01.2020 02:49, Alexander Popov wrote:
> Make the stack erasing test more verbose about the errors that it
> can detect.
> 
> Signed-off-by: Alexander Popov <alex.popov@linux.com>
> ---
>  drivers/misc/lkdtm/stackleak.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)

Hello!

Pinging about this version of the patch.

Kees, it uses dump_stack() instead of BUG(), as we negotiated.

Thanks!
Alexander

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

* Re: [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose
  2020-01-22 11:58 ` Alexander Popov
@ 2020-01-27 23:15   ` Kees Cook
  2020-01-28  7:50     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2020-01-27 23:15 UTC (permalink / raw)
  To: Alexander Popov; +Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, notify

On Wed, Jan 22, 2020 at 02:58:44PM +0300, Alexander Popov wrote:
> On 03.01.2020 02:49, Alexander Popov wrote:
> > Make the stack erasing test more verbose about the errors that it
> > can detect.
> > 
> > Signed-off-by: Alexander Popov <alex.popov@linux.com>
> > ---
> >  drivers/misc/lkdtm/stackleak.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> Hello!
> 
> Pinging about this version of the patch.
> 
> Kees, it uses dump_stack() instead of BUG(), as we negotiated.

Yup, this is in my queue -- I've just gotten back from travelling and
will get to it shortly. :)

Greg, feel free to take this directly if you want, too.

-Kees

-- 
Kees Cook

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

* Re: [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose
  2020-01-27 23:15   ` Kees Cook
@ 2020-01-28  7:50     ` Greg Kroah-Hartman
  2020-01-28 23:03       ` Kees Cook
  0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-28  7:50 UTC (permalink / raw)
  To: Kees Cook; +Cc: Alexander Popov, Arnd Bergmann, linux-kernel, notify

On Mon, Jan 27, 2020 at 03:15:08PM -0800, Kees Cook wrote:
> On Wed, Jan 22, 2020 at 02:58:44PM +0300, Alexander Popov wrote:
> > On 03.01.2020 02:49, Alexander Popov wrote:
> > > Make the stack erasing test more verbose about the errors that it
> > > can detect.
> > > 
> > > Signed-off-by: Alexander Popov <alex.popov@linux.com>
> > > ---
> > >  drivers/misc/lkdtm/stackleak.c | 25 +++++++++++++++++--------
> > >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > Hello!
> > 
> > Pinging about this version of the patch.
> > 
> > Kees, it uses dump_stack() instead of BUG(), as we negotiated.
> 
> Yup, this is in my queue -- I've just gotten back from travelling and
> will get to it shortly. :)
> 
> Greg, feel free to take this directly if you want, too.

If I were to take it, it would have to wait until after 5.6-rc1, sorry.

thanks,

greg k-h

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

* Re: [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose
  2020-01-28  7:50     ` Greg Kroah-Hartman
@ 2020-01-28 23:03       ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2020-01-28 23:03 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alexander Popov, Arnd Bergmann, linux-kernel, notify

On Tue, Jan 28, 2020 at 08:50:50AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 27, 2020 at 03:15:08PM -0800, Kees Cook wrote:
> > On Wed, Jan 22, 2020 at 02:58:44PM +0300, Alexander Popov wrote:
> > > On 03.01.2020 02:49, Alexander Popov wrote:
> > > > Make the stack erasing test more verbose about the errors that it
> > > > can detect.
> > > > 
> > > > Signed-off-by: Alexander Popov <alex.popov@linux.com>
> > > > ---
> > > >  drivers/misc/lkdtm/stackleak.c | 25 +++++++++++++++++--------
> > > >  1 file changed, 17 insertions(+), 8 deletions(-)
> > > 
> > > Hello!
> > > 
> > > Pinging about this version of the patch.
> > > 
> > > Kees, it uses dump_stack() instead of BUG(), as we negotiated.
> > 
> > Yup, this is in my queue -- I've just gotten back from travelling and
> > will get to it shortly. :)
> > 
> > Greg, feel free to take this directly if you want, too.
> 
> If I were to take it, it would have to wait until after 5.6-rc1, sorry.

Well, it has to go via drivers/misc anyway, so I think timing is no
different. I would consider this a feature patch, not a bug fix, too.

-- 
Kees Cook

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

end of thread, other threads:[~2020-01-28 23:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-02 23:49 [PATCH v2 1/1] lkdtm/stackleak: Make the test more verbose Alexander Popov
2020-01-14 12:14 ` Alexander Popov
2020-01-22 11:58 ` Alexander Popov
2020-01-27 23:15   ` Kees Cook
2020-01-28  7:50     ` Greg Kroah-Hartman
2020-01-28 23:03       ` Kees Cook

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