On Wed, 5 Jun 2019 at 07:25, Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > While the gcc plugin for automatic stack variable initialization (i.e. > CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL) performs initialization with > 0x00 bytes, the Clang automatic stack variable initialization (i.e. > CONFIG_INIT_STACK_ALL) uses various type-specific patterns that are > typically 0xAA. Therefore the stackinit selftest has been fixed to check > that bytes are no longer the test fill pattern of 0xFF (instead of looking > for bytes that have become 0x00). This retains the test coverage for the > 0x00 pattern of the gcc plugin while adding coverage for the mostly 0xAA > pattern of Clang. > > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> Acked-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > --- > lib/test_stackinit.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/lib/test_stackinit.c b/lib/test_stackinit.c > index e97dc54b4fdf..2d7d257a430e 100644 > --- a/lib/test_stackinit.c > +++ b/lib/test_stackinit.c > @@ -12,7 +12,7 @@ > > /* Exfiltration buffer. */ > #define MAX_VAR_SIZE 128 > -static char check_buf[MAX_VAR_SIZE]; > +static u8 check_buf[MAX_VAR_SIZE]; > > /* Character array to trigger stack protector in all functions. */ > #define VAR_BUFFER 32 > @@ -106,9 +106,18 @@ static noinline __init int test_ ## name (void) \ > \ > /* Fill clone type with zero for per-field init. */ \ > memset(&zero, 0x00, sizeof(zero)); \ > + /* Clear entire check buffer for 0xFF overlap test. */ \ > + memset(check_buf, 0x00, sizeof(check_buf)); \ > /* Fill stack with 0xFF. */ \ > ignored = leaf_ ##name((unsigned long)&ignored, 1, \ > FETCH_ARG_ ## which(zero)); \ > + /* Verify all bytes overwritten with 0xFF. */ \ > + for (sum = 0, i = 0; i < target_size; i++) \ > + sum += (check_buf[i] != 0xFF); \ > + if (sum) { \ > + pr_err(#name ": leaf fill was not 0xFF!?\n"); \ > + return 1; \ > + } \ > /* Clear entire check buffer for later bit tests. */ \ > memset(check_buf, 0x00, sizeof(check_buf)); \ > /* Extract stack-defined variable contents. */ \ > @@ -126,9 +135,9 @@ static noinline __init int test_ ## name (void) \ > return 1; \ > } \ > \ > - /* Look for any set bits in the check region. */ \ > - for (i = 0; i < sizeof(check_buf); i++) \ > - sum += (check_buf[i] != 0); \ > + /* Look for any bytes still 0xFF in check region. */ \ > + for (sum = 0, i = 0; i < target_size; i++) \ > + sum += (check_buf[i] == 0xFF); \ > \ > if (sum == 0) \ > pr_info(#name " ok\n"); \ > @@ -162,13 +171,13 @@ static noinline __init int leaf_ ## name(unsigned long sp, \ > * Keep this buffer around to make sure we've got a \ > * stack frame of SOME kind... \ > */ \ > - memset(buf, (char)(sp && 0xff), sizeof(buf)); \ > + memset(buf, (char)(sp & 0xff), sizeof(buf)); \ > /* Fill variable with 0xFF. */ \ > if (fill) { \ > fill_start = &var; \ > fill_size = sizeof(var); \ > memset(fill_start, \ > - (char)((sp && 0xff) | forced_mask), \ > + (char)((sp & 0xff) | forced_mask), \ > fill_size); \ > } \ > \ > -- > 2.17.1 > > > -- > Kees Cook