From 95925c99b9043d52db626645e6ef5ee5f62c97e4 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 24 Apr 2017 13:23:21 -0700 Subject: [PATCH] lkdtm: Provide more complete coverage for REFCOUNT tests The existing REFCOUNT_* LKDTM tests were designed only for testing a narrow portion of CONFIG_REFCOUNT_FULL. This moves the tests to their own file and expands their testing to poke each boundary condition. Since the protections (CONFIG_REFCOUNT_FULL and x86-fast) use different saturation values and reach-zero behavior, those have to be build-time set so the tests can actually validate things are happening at the right places. Notably, the x86-fast protection will fail REFCOUNT_INC_ZERO and REFCOUNT_ADD_ZERO since those conditions are not checked (only overflow is critical to protecting refcount_t). CONFIG_REFCOUNT_FULL will warn for each REFCOUNT_*_NEGATIVE test since it provides zero-pinning behaviors (which allows it to pass REFCOUNT_INC_ZERO and REFCOUNT_ADD_ZERO). Signed-off-by: Kees Cook --- drivers/misc/Makefile | 1 + drivers/misc/lkdtm.h | 25 ++- drivers/misc/lkdtm_bugs.c | 83 -------- drivers/misc/lkdtm_core.c | 23 ++- drivers/misc/lkdtm_refcount.c | 356 ++++++++++++++++++++++++++++++++++ 5 files changed, 393 insertions(+), 95 deletions(-) create mode 100644 drivers/misc/lkdtm_refcount.c diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index b0b766416306..d84819dc2468 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -60,6 +60,7 @@ lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o lkdtm-$(CONFIG_LKDTM) += lkdtm_heap.o lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o +lkdtm-$(CONFIG_LKDTM) += lkdtm_refcount.o lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h index 3b4976396ec4..04ff8b23b3b0 100644 --- a/drivers/misc/lkdtm.h +++ b/drivers/misc/lkdtm.h @@ -19,12 +19,6 @@ void lkdtm_SOFTLOCKUP(void); void lkdtm_HARDLOCKUP(void); void lkdtm_SPINLOCKUP(void); void lkdtm_HUNG_TASK(void); -void lkdtm_REFCOUNT_SATURATE_INC(void); -void lkdtm_REFCOUNT_SATURATE_ADD(void); -void lkdtm_REFCOUNT_ZERO_DEC(void); -void lkdtm_REFCOUNT_ZERO_INC(void); -void lkdtm_REFCOUNT_ZERO_SUB(void); -void lkdtm_REFCOUNT_ZERO_ADD(void); void lkdtm_CORRUPT_LIST_ADD(void); void lkdtm_CORRUPT_LIST_DEL(void); void lkdtm_CORRUPT_USER_DS(void); @@ -49,6 +43,25 @@ void lkdtm_EXEC_RODATA(void); void lkdtm_EXEC_USERSPACE(void); void lkdtm_ACCESS_USERSPACE(void); +/* lkdtm_refcount.c */ +void lkdtm_REFCOUNT_INC_OVERFLOW(void); +void lkdtm_REFCOUNT_ADD_OVERFLOW(void); +void lkdtm_REFCOUNT_INC_NOT_ZERO_OVERFLOW(void); +void lkdtm_REFCOUNT_ADD_NOT_ZERO_OVERFLOW(void); +void lkdtm_REFCOUNT_DEC_ZERO(void); +void lkdtm_REFCOUNT_DEC_NEGATIVE(void); +void lkdtm_REFCOUNT_DEC_AND_TEST_NEGATIVE(void); +void lkdtm_REFCOUNT_SUB_AND_TEST_NEGATIVE(void); +void lkdtm_REFCOUNT_INC_ZERO(void); +void lkdtm_REFCOUNT_ADD_ZERO(void); +void lkdtm_REFCOUNT_INC_SATURATED(void); +void lkdtm_REFCOUNT_DEC_SATURATED(void); +void lkdtm_REFCOUNT_ADD_SATURATED(void); +void lkdtm_REFCOUNT_INC_NOT_ZERO_SATURATED(void); +void lkdtm_REFCOUNT_ADD_NOT_ZERO_SATURATED(void); +void lkdtm_REFCOUNT_DEC_AND_TEST_SATURATED(void); +void lkdtm_REFCOUNT_SUB_AND_TEST_SATURATED(void); + /* lkdtm_rodata.c */ void lkdtm_rodata_do_nothing(void); diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c index d9028ef50fbe..ef3d06f901fc 100644 --- a/drivers/misc/lkdtm_bugs.c +++ b/drivers/misc/lkdtm_bugs.c @@ -6,7 +6,6 @@ */ #include "lkdtm.h" #include -#include #include #include #include @@ -137,88 +136,6 @@ void lkdtm_HUNG_TASK(void) schedule(); } -void lkdtm_REFCOUNT_SATURATE_INC(void) -{ - refcount_t over = REFCOUNT_INIT(UINT_MAX - 1); - - pr_info("attempting good refcount decrement\n"); - refcount_dec(&over); - refcount_inc(&over); - - pr_info("attempting bad refcount inc overflow\n"); - refcount_inc(&over); - refcount_inc(&over); - if (refcount_read(&over) == UINT_MAX) - pr_err("Correctly stayed saturated, but no BUG?!\n"); - else - pr_err("Fail: refcount wrapped\n"); -} - -void lkdtm_REFCOUNT_SATURATE_ADD(void) -{ - refcount_t over = REFCOUNT_INIT(UINT_MAX - 1); - - pr_info("attempting good refcount decrement\n"); - refcount_dec(&over); - refcount_inc(&over); - - pr_info("attempting bad refcount add overflow\n"); - refcount_add(2, &over); - if (refcount_read(&over) == UINT_MAX) - pr_err("Correctly stayed saturated, but no BUG?!\n"); - else - pr_err("Fail: refcount wrapped\n"); -} - -void lkdtm_REFCOUNT_ZERO_DEC(void) -{ - refcount_t zero = REFCOUNT_INIT(1); - - pr_info("attempting bad refcount decrement to zero\n"); - refcount_dec(&zero); - if (refcount_read(&zero) == 0) - pr_err("Stayed at zero, but no BUG?!\n"); - else - pr_err("Fail: refcount went crazy\n"); -} - -void lkdtm_REFCOUNT_ZERO_SUB(void) -{ - refcount_t zero = REFCOUNT_INIT(1); - - pr_info("attempting bad refcount subtract past zero\n"); - if (!refcount_sub_and_test(2, &zero)) - pr_info("wrap attempt was noticed\n"); - if (refcount_read(&zero) == 1) - pr_err("Correctly stayed above 0, but no BUG?!\n"); - else - pr_err("Fail: refcount wrapped\n"); -} - -void lkdtm_REFCOUNT_ZERO_INC(void) -{ - refcount_t zero = REFCOUNT_INIT(0); - - pr_info("attempting bad refcount increment from zero\n"); - refcount_inc(&zero); - if (refcount_read(&zero) == 0) - pr_err("Stayed at zero, but no BUG?!\n"); - else - pr_err("Fail: refcount went past zero\n"); -} - -void lkdtm_REFCOUNT_ZERO_ADD(void) -{ - refcount_t zero = REFCOUNT_INIT(0); - - pr_info("attempting bad refcount addition from zero\n"); - refcount_add(2, &zero); - if (refcount_read(&zero) == 0) - pr_err("Stayed at zero, but no BUG?!\n"); - else - pr_err("Fail: refcount went past zero\n"); -} - void lkdtm_CORRUPT_LIST_ADD(void) { /* diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c index 42d2b8e31e6b..156a1a07c3db 100644 --- a/drivers/misc/lkdtm_core.c +++ b/drivers/misc/lkdtm_core.c @@ -221,12 +221,23 @@ struct crashtype crashtypes[] = { CRASHTYPE(WRITE_RO), CRASHTYPE(WRITE_RO_AFTER_INIT), CRASHTYPE(WRITE_KERN), - CRASHTYPE(REFCOUNT_SATURATE_INC), - CRASHTYPE(REFCOUNT_SATURATE_ADD), - CRASHTYPE(REFCOUNT_ZERO_DEC), - CRASHTYPE(REFCOUNT_ZERO_INC), - CRASHTYPE(REFCOUNT_ZERO_SUB), - CRASHTYPE(REFCOUNT_ZERO_ADD), + CRASHTYPE(REFCOUNT_INC_OVERFLOW), + CRASHTYPE(REFCOUNT_ADD_OVERFLOW), + CRASHTYPE(REFCOUNT_INC_NOT_ZERO_OVERFLOW), + CRASHTYPE(REFCOUNT_ADD_NOT_ZERO_OVERFLOW), + CRASHTYPE(REFCOUNT_DEC_ZERO), + CRASHTYPE(REFCOUNT_DEC_NEGATIVE), + CRASHTYPE(REFCOUNT_DEC_AND_TEST_NEGATIVE), + CRASHTYPE(REFCOUNT_SUB_AND_TEST_NEGATIVE), + CRASHTYPE(REFCOUNT_INC_ZERO), + CRASHTYPE(REFCOUNT_ADD_ZERO), + CRASHTYPE(REFCOUNT_INC_SATURATED), + CRASHTYPE(REFCOUNT_DEC_SATURATED), + CRASHTYPE(REFCOUNT_ADD_SATURATED), + CRASHTYPE(REFCOUNT_INC_NOT_ZERO_SATURATED), + CRASHTYPE(REFCOUNT_ADD_NOT_ZERO_SATURATED), + CRASHTYPE(REFCOUNT_DEC_AND_TEST_SATURATED), + CRASHTYPE(REFCOUNT_SUB_AND_TEST_SATURATED), CRASHTYPE(USERCOPY_HEAP_SIZE_TO), CRASHTYPE(USERCOPY_HEAP_SIZE_FROM), CRASHTYPE(USERCOPY_HEAP_FLAG_TO), diff --git a/drivers/misc/lkdtm_refcount.c b/drivers/misc/lkdtm_refcount.c new file mode 100644 index 000000000000..313abea4bf9d --- /dev/null +++ b/drivers/misc/lkdtm_refcount.c @@ -0,0 +1,356 @@ +/* + * This is for all the tests related to refcount bugs (e.g. overflow, + * underflow, reaching zero untested, etc). + */ +#include "lkdtm.h" +#include + +#ifdef CONFIG_REFCOUNT_FULL +#define REFCOUNT_MAX (UINT_MAX - 1) +#define REFCOUNT_SATURATED UINT_MAX +#else +#define REFCOUNT_MAX INT_MAX +#define REFCOUNT_SATURATED (INT_MIN / 2) +#endif + +static void overflow_check(refcount_t *ref) +{ + switch (refcount_read(ref)) { + case REFCOUNT_SATURATED: + pr_info("Overflow detected: saturated\n"); + break; + case REFCOUNT_MAX: + pr_warn("Overflow detected: unsafely reset to max\n"); + break; + default: + pr_err("Fail: refcount wrapped to %d\n", refcount_read(ref)); + } +} + +/* + * A refcount_inc() above the maximum value of the refcount implementation, + * should at least saturate, and at most also WARN. + */ +void lkdtm_REFCOUNT_INC_OVERFLOW(void) +{ + refcount_t over = REFCOUNT_INIT(REFCOUNT_MAX - 1); + + pr_info("attempting good refcount_inc() without overflow\n"); + refcount_dec(&over); + refcount_inc(&over); + + pr_info("attempting bad refcount_inc() overflow\n"); + refcount_inc(&over); + refcount_inc(&over); + + overflow_check(&over); +} + +/* refcount_add() should behave just like refcount_inc() above. */ +void lkdtm_REFCOUNT_ADD_OVERFLOW(void) +{ + refcount_t over = REFCOUNT_INIT(REFCOUNT_MAX - 1); + + pr_info("attempting good refcount_add() without overflow\n"); + refcount_dec(&over); + refcount_dec(&over); + refcount_dec(&over); + refcount_dec(&over); + refcount_add(4, &over); + + pr_info("attempting bad refcount_add() overflow\n"); + refcount_add(4, &over); + + overflow_check(&over); +} + +/* refcount_inc_not_zero() should behave just like refcount_inc() above. */ +void lkdtm_REFCOUNT_INC_NOT_ZERO_OVERFLOW(void) +{ + refcount_t over = REFCOUNT_INIT(REFCOUNT_MAX); + + pr_info("attempting bad refcount_inc_not_zero() overflow\n"); + if (!refcount_inc_not_zero(&over)) + pr_warn("Weird: refcount_inc_not_zero() reported zero\n"); + + overflow_check(&over); +} + +/* refcount_add_not_zero() should behave just like refcount_inc() above. */ +void lkdtm_REFCOUNT_ADD_NOT_ZERO_OVERFLOW(void) +{ + refcount_t over = REFCOUNT_INIT(REFCOUNT_MAX); + + pr_info("attempting bad refcount_add_not_zero() overflow\n"); + if (!refcount_add_not_zero(6, &over)) + pr_warn("Weird: refcount_add_not_zero() reported zero\n"); + + overflow_check(&over); +} + +static void check_zero(refcount_t *ref) +{ + switch (refcount_read(ref)) { + case REFCOUNT_SATURATED: + pr_info("Zero detected: saturated\n"); + break; + case REFCOUNT_MAX: + pr_warn("Zero detected: unsafely reset to max\n"); + break; + case 0: + pr_warn("Still at zero: refcount_inc/add() must not inc-from-0\n"); + break; + default: + pr_err("Fail: refcount went crazy: %d\n", refcount_read(ref)); + } +} + +/* + * A refcount_dec(), as opposed to a refcount_dec_and_test(), when it hits + * zero it should either saturate (when inc-from-zero isn't protected) + * or stay at zero (when inc-from-zero is protected) and should WARN for both. + */ +void lkdtm_REFCOUNT_DEC_ZERO(void) +{ + refcount_t zero = REFCOUNT_INIT(2); + + pr_info("attempting good refcount_dec()\n"); + refcount_dec(&zero); + + pr_info("attempting bad refcount_dec() to zero\n"); + refcount_dec(&zero); + + check_zero(&zero); +} + +static void check_negative(refcount_t *ref, int start) +{ + /* + * CONFIG_REFCOUNT_FULL refuses to move a refcount at all on an + * over-sub, so we have to track our starting position instead of + * looking only at zero-pinning. + */ + if (refcount_read(ref) == start) { + pr_warn("Still at %d: refcount_inc/add() must not inc-from-0\n", + start); + return; + } + + switch (refcount_read(ref)) { + case REFCOUNT_SATURATED: + pr_info("Negative detected: saturated\n"); + break; + case REFCOUNT_MAX: + pr_warn("Negative detected: unsafely reset to max\n"); + break; + default: + pr_err("Fail: refcount went crazy: %d\n", refcount_read(ref)); + } +} + +/* A refcount_dec() going negative should saturate and may WARN. */ +void lkdtm_REFCOUNT_DEC_NEGATIVE(void) +{ + refcount_t neg = REFCOUNT_INIT(0); + + pr_info("attempting bad refcount_dec() below zero\n"); + refcount_dec(&neg); + + check_negative(&neg, 0); +} + +/* + * A refcount_dec_and_test() should act like refcount_dec() above when + * going negative. + */ +void lkdtm_REFCOUNT_DEC_AND_TEST_NEGATIVE(void) +{ + refcount_t neg = REFCOUNT_INIT(0); + + pr_info("attempting bad refcount_dec_and_test() below zero\n"); + if (refcount_dec_and_test(&neg)) + pr_warn("Weird: refcount_dec_and_test() reported zero\n"); + + check_negative(&neg, 0); +} + +/* + * A refcount_sub_and_test() should act like refcount_dec_and_test() + * above when going negative. + */ +void lkdtm_REFCOUNT_SUB_AND_TEST_NEGATIVE(void) +{ + refcount_t neg = REFCOUNT_INIT(3); + + pr_info("attempting bad refcount_sub_and_test() below zero\n"); + if (refcount_sub_and_test(5, &neg)) + pr_warn("Weird: refcount_sub_and_test() reported zero\n"); + + check_negative(&neg, 3); +} + +static void check_from_zero(refcount_t *ref) +{ + switch (refcount_read(ref)) { + case 0: + pr_info("Zero detected: stayed at zero\n"); + break; + case REFCOUNT_SATURATED: + pr_info("Zero detected: saturated\n"); + break; + case REFCOUNT_MAX: + pr_warn("Zero detected: unsafely reset to max\n"); + break; + default: + pr_info("Fail: zero not detected, incremeted to %d\n", + refcount_read(ref)); + } +} + +/* + * A refcount_inc() from zero should pin to zero or saturate and may WARN. + * Only CONFIG_REFCOUNT_FULL provides this protection currently. + */ +void lkdtm_REFCOUNT_INC_ZERO(void) +{ + refcount_t zero = REFCOUNT_INIT(0); + + pr_info("attempting safe refcount_inc_not_zero() from zero\n"); + if (!refcount_inc_not_zero(&zero)) { + pr_info("Good: zero detected\n"); + if (refcount_read(&zero) == 0) + pr_info("Correctly stayed at zero\n"); + else + pr_err("Fail: refcount went past zero!\n"); + } else { + pr_err("Fail: Zero not detected!?\n"); + } + + pr_info("attempting bad refcount_inc() from zero\n"); + refcount_inc(&zero); + + check_from_zero(&zero); +} + +/* + * A refcount_add() should act like refcount_inc() above when starting + * at zero. + */ +void lkdtm_REFCOUNT_ADD_ZERO(void) +{ + refcount_t zero = REFCOUNT_INIT(0); + + pr_info("attempting safe refcount_add_not_zero() from zero\n"); + if (!refcount_add_not_zero(3, &zero)) { + pr_info("Good: zero detected\n"); + if (refcount_read(&zero) == 0) + pr_info("Correctly stayed at zero\n"); + else + pr_err("Fail: refcount went past zero\n"); + } else { + pr_err("Fail: Zero not detected!?\n"); + } + + pr_info("attempting bad refcount_add() from zero\n"); + refcount_add(3, &zero); + + check_from_zero(&zero); +} + +static void check_saturated(refcount_t *ref) +{ + switch (refcount_read(ref)) { + case REFCOUNT_SATURATED: + pr_info("Saturation detected: still saturated\n"); + break; + case REFCOUNT_MAX: + pr_warn("Saturation detected: unsafely reset to max\n"); + break; + default: + pr_err("Fail: refcount went crazy: %d\n", refcount_read(ref)); + } +} + +/* + * A refcount_inc() from a saturated value should at most warn about + * being saturated already. + */ +void lkdtm_REFCOUNT_INC_SATURATED(void) +{ + refcount_t sat = REFCOUNT_INIT(REFCOUNT_SATURATED); + + pr_info("attempting bad refcount_inc() from saturated\n"); + refcount_inc(&sat); + + check_saturated(&sat); +} + +/* Should act like refcount_inc() above from saturated. */ +void lkdtm_REFCOUNT_DEC_SATURATED(void) +{ + refcount_t sat = REFCOUNT_INIT(REFCOUNT_SATURATED); + + pr_info("attempting bad refcount_dec() from saturated\n"); + refcount_dec(&sat); + + check_saturated(&sat); +} + +/* Should act like refcount_inc() above from saturated. */ +void lkdtm_REFCOUNT_ADD_SATURATED(void) +{ + refcount_t sat = REFCOUNT_INIT(REFCOUNT_SATURATED); + + pr_info("attempting bad refcount_dec() from saturated\n"); + refcount_add(8, &sat); + + check_saturated(&sat); +} + +/* Should act like refcount_inc() above from saturated. */ +void lkdtm_REFCOUNT_INC_NOT_ZERO_SATURATED(void) +{ + refcount_t sat = REFCOUNT_INIT(REFCOUNT_SATURATED); + + pr_info("attempting bad refcount_inc_not_zero() from saturated\n"); + if (!refcount_inc_not_zero(&sat)) + pr_warn("Weird: refcount_inc_not_zero() reported zero\n"); + + check_saturated(&sat); +} + +/* Should act like refcount_inc() above from saturated. */ +void lkdtm_REFCOUNT_ADD_NOT_ZERO_SATURATED(void) +{ + refcount_t sat = REFCOUNT_INIT(REFCOUNT_SATURATED); + + pr_info("attempting bad refcount_add_not_zero() from saturated\n"); + if (!refcount_add_not_zero(7, &sat)) + pr_warn("Weird: refcount_add_not_zero() reported zero\n"); + + check_saturated(&sat); +} + +/* Should act like refcount_inc() above from saturated. */ +void lkdtm_REFCOUNT_DEC_AND_TEST_SATURATED(void) +{ + refcount_t sat = REFCOUNT_INIT(REFCOUNT_SATURATED); + + pr_info("attempting bad refcount_dec_and_test() from saturated\n"); + if (refcount_dec_and_test(&sat)) + pr_warn("Weird: refcount_dec_and_test() reported zero\n"); + + check_saturated(&sat); +} + +/* Should act like refcount_inc() above from saturated. */ +void lkdtm_REFCOUNT_SUB_AND_TEST_SATURATED(void) +{ + refcount_t sat = REFCOUNT_INIT(REFCOUNT_SATURATED); + + pr_info("attempting bad refcount_sub_and_test() from saturated\n"); + if (refcount_sub_and_test(8, &sat)) + pr_warn("Weird: refcount_sub_and_test() reported zero\n"); + + check_saturated(&sat); +}