linux-kernel - Re: [PATCH v1 1/1] lkdtm/stackleak: Make the stack erasing test more verbose

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: <7d692333-7ff8-0c3f-8e36-dd2cc0ff3163@linux.com>
Date:   Sat, 28 Dec 2019 23:20:47 +0300
From:   Alexander Popov <alex.popov@...ux.com>
To:     Kees Cook <keescook@...omium.org>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org
Cc:     notify@...nel.org
Subject: Re: [PATCH v1 1/1] lkdtm/stackleak: Make the stack erasing test more
 verbose

Hello!

Just a friendly ping.
Could I have the feedback for this patch?

Best regards,
Alexander


On 19.12.2019 17:54, Alexander Popov wrote:
> Make the stack erasing test more verbose about the errors that it
> can detect. BUG() in case of test failure is useful when the test
> is running in a loop.
> 
> Signed-off-by: Alexander Popov <alex.popov@...ux.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..d198de4d4c7e 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");
> +		BUG();
> +	} else {
> +		pr_info("OK: the rest of the thread stack is properly erased\n");
> +	}
>  }
>