From 547840bd5ae52a9d864abddcb91ea491a78d8199 Mon Sep 17 00:00:00 2001 From: Joe Lawrence Date: Wed, 3 Jun 2020 14:20:55 -0400 Subject: [PATCH 1/9] selftests/livepatch: simplify test-klp-callbacks busy target tests The test-klp-callbacks script includes a few tests which rely on kernel task timings that may not always execute as expected under system load. These may generate out of sequence kernel log messages that result in test failure. Instead of using sleep timing windows to orchestrate these tests, add a block_transition module parameter to communicate the test purpose and utilize flush_queue() to serialize the test module's task output. Signed-off-by: Joe Lawrence Reviewed-by: Petr Mladek Acked-by: Miroslav Benes Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200603182058.109470-2-ycote@redhat.com --- lib/livepatch/test_klp_callbacks_busy.c | 37 ++++++++++++++----- .../selftests/livepatch/test-callbacks.sh | 29 +++++++-------- 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/lib/livepatch/test_klp_callbacks_busy.c b/lib/livepatch/test_klp_callbacks_busy.c index 40beddf8a0e2..7ac845f65be5 100644 --- a/lib/livepatch/test_klp_callbacks_busy.c +++ b/lib/livepatch/test_klp_callbacks_busy.c @@ -5,34 +5,53 @@ #include #include +#include #include #include -static int sleep_secs; -module_param(sleep_secs, int, 0644); -MODULE_PARM_DESC(sleep_secs, "sleep_secs (default=0)"); +/* load/run-time control from sysfs writer */ +static bool block_transition; +module_param(block_transition, bool, 0644); +MODULE_PARM_DESC(block_transition, "block_transition (default=false)"); static void busymod_work_func(struct work_struct *work); -static DECLARE_DELAYED_WORK(work, busymod_work_func); +static DECLARE_WORK(work, busymod_work_func); static void busymod_work_func(struct work_struct *work) { - pr_info("%s, sleeping %d seconds ...\n", __func__, sleep_secs); - msleep(sleep_secs * 1000); + pr_info("%s enter\n", __func__); + + while (READ_ONCE(block_transition)) { + /* + * Busy-wait until the sysfs writer has acknowledged a + * blocked transition and clears the flag. + */ + msleep(20); + } + pr_info("%s exit\n", __func__); } static int test_klp_callbacks_busy_init(void) { pr_info("%s\n", __func__); - schedule_delayed_work(&work, - msecs_to_jiffies(1000 * 0)); + schedule_work(&work); + + if (!block_transition) { + /* + * Serialize output: print all messages from the work + * function before returning from init(). + */ + flush_work(&work); + } + return 0; } static void test_klp_callbacks_busy_exit(void) { - cancel_delayed_work_sync(&work); + WRITE_ONCE(block_transition, false); + flush_work(&work); pr_info("%s\n", __func__); } diff --git a/tools/testing/selftests/livepatch/test-callbacks.sh b/tools/testing/selftests/livepatch/test-callbacks.sh index a35289b13c9c..32b57ba07f4f 100755 --- a/tools/testing/selftests/livepatch/test-callbacks.sh +++ b/tools/testing/selftests/livepatch/test-callbacks.sh @@ -356,9 +356,7 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete echo -n "TEST: multiple target modules ... " dmesg -C -load_mod $MOD_TARGET_BUSY sleep_secs=0 -# give $MOD_TARGET_BUSY::busymod_work_func() a chance to run -sleep 5 +load_mod $MOD_TARGET_BUSY block_transition=N load_lp $MOD_LIVEPATCH load_mod $MOD_TARGET unload_mod $MOD_TARGET @@ -366,9 +364,9 @@ disable_lp $MOD_LIVEPATCH unload_lp $MOD_LIVEPATCH unload_mod $MOD_TARGET_BUSY -check_result "% modprobe $MOD_TARGET_BUSY sleep_secs=0 +check_result "% modprobe $MOD_TARGET_BUSY block_transition=N $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_init -$MOD_TARGET_BUSY: busymod_work_func, sleeping 0 seconds ... +$MOD_TARGET_BUSY: busymod_work_func enter $MOD_TARGET_BUSY: busymod_work_func exit % modprobe $MOD_LIVEPATCH livepatch: enabling patch '$MOD_LIVEPATCH' @@ -404,11 +402,10 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit" - # TEST: busy target module # # A similar test as the previous one, but force the "busy" kernel module -# to do longer work. +# to block the livepatch transition. # # The livepatching core will refuse to patch a task that is currently # executing a to-be-patched function -- the consistency model stalls the @@ -417,8 +414,7 @@ $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit" # function for a long time. Meanwhile, load and unload other target # kernel modules while the livepatch transition is in progress. # -# - Load the "busy" kernel module, this time make it do 10 seconds worth -# of work. +# - Load the "busy" kernel module, this time make its work function loop # # - Meanwhile, the livepatch is loaded. Notice that the patch # transition does not complete as the targeted "busy" module is @@ -438,20 +434,23 @@ $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit" echo -n "TEST: busy target module ... " dmesg -C -load_mod $MOD_TARGET_BUSY sleep_secs=10 +load_mod $MOD_TARGET_BUSY block_transition=Y load_lp_nowait $MOD_LIVEPATCH -# Don't wait for transition, load $MOD_TARGET while the transition -# is still stalled in $MOD_TARGET_BUSY::busymod_work_func() -sleep 5 + +# Wait until the livepatch reports in-transition state, i.e. that it's +# stalled on $MOD_TARGET_BUSY::busymod_work_func() +loop_until 'grep -q '^1$' /sys/kernel/livepatch/$MOD_LIVEPATCH/transition' || + die "failed to stall transition" + load_mod $MOD_TARGET unload_mod $MOD_TARGET disable_lp $MOD_LIVEPATCH unload_lp $MOD_LIVEPATCH unload_mod $MOD_TARGET_BUSY -check_result "% modprobe $MOD_TARGET_BUSY sleep_secs=10 +check_result "% modprobe $MOD_TARGET_BUSY block_transition=Y $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_init -$MOD_TARGET_BUSY: busymod_work_func, sleeping 10 seconds ... +$MOD_TARGET_BUSY: busymod_work_func enter % modprobe $MOD_LIVEPATCH livepatch: enabling patch '$MOD_LIVEPATCH' livepatch: '$MOD_LIVEPATCH': initializing patching transition From 6a26a9df169d81aba244d6496e4381401d04e281 Mon Sep 17 00:00:00 2001 From: Yannick Cote Date: Wed, 3 Jun 2020 14:20:56 -0400 Subject: [PATCH 2/9] selftests/livepatch: rework test-klp-shadow-vars The initial idea was to make a change to please cppcheck and remove void pointer arithmetics found a few times: portability: 'obj' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. [arithOperationsOnVoidPointer] The rest of the changes are to help make the test read as an example while continuing to verify the shadow variable code. The logic of the test is unchanged but restructured to use descriptive names. Signed-off-by: Yannick Cote Reviewed-by: Petr Mladek Reviewed-by: Kamalesh Babulal Acked-by: Miroslav Benes Acked-by: Joe Lawrence Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200603182058.109470-3-ycote@redhat.com --- lib/livepatch/test_klp_shadow_vars.c | 101 +++++++++++++++------------ 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/lib/livepatch/test_klp_shadow_vars.c b/lib/livepatch/test_klp_shadow_vars.c index f0b5a1d24e55..ec2635cff974 100644 --- a/lib/livepatch/test_klp_shadow_vars.c +++ b/lib/livepatch/test_klp_shadow_vars.c @@ -109,8 +109,7 @@ static void shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor) static void shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor) { klp_shadow_free_all(id, dtor); - pr_info("klp_%s(id=0x%lx, dtor=PTR%d)\n", - __func__, id, ptr_id(dtor)); + pr_info("klp_%s(id=0x%lx, dtor=PTR%d)\n", __func__, id, ptr_id(dtor)); } @@ -124,8 +123,7 @@ static int shadow_ctor(void *obj, void *shadow_data, void *ctor_data) return -EINVAL; *sv = *var; - pr_info("%s: PTR%d -> PTR%d\n", - __func__, ptr_id(sv), ptr_id(*var)); + pr_info("%s: PTR%d -> PTR%d\n", __func__, ptr_id(sv), ptr_id(*var)); return 0; } @@ -138,49 +136,63 @@ static void shadow_dtor(void *obj, void *shadow_data) __func__, ptr_id(obj), ptr_id(sv)); } +/* dynamically created obj fields have the following shadow var id values */ +#define SV_ID1 0x1234 +#define SV_ID2 0x1235 + +/* + * The main test case adds/removes new fields (shadow var) to each of these + * test structure instances. The last group of fields in the struct represent + * the idea that shadow variables may be added and removed to and from the + * struct during execution. + */ +struct test_object { + /* add anything here below and avoid to define an empty struct */ + struct shadow_ptr sp; + + /* these represent shadow vars added and removed with SV_ID{1,2} */ + /* char nfield1; */ + /* int nfield2; */ +}; + static int test_klp_shadow_vars_init(void) { - void *obj = THIS_MODULE; - int id = 0x1234; - gfp_t gfp_flags = GFP_KERNEL; + struct test_object obj1, obj2, obj3; + char nfield1, nfield2, *pnfield1, *pnfield2, **sv1, **sv2; + int nfield3, nfield4, *pnfield3, *pnfield4, **sv3, **sv4; + void **sv; - int var1, var2, var3, var4; - int *pv1, *pv2, *pv3, *pv4; - int **sv1, **sv2, **sv3, **sv4; - - int **sv; - - pv1 = &var1; - pv2 = &var2; - pv3 = &var3; - pv4 = &var4; + pnfield1 = &nfield1; + pnfield2 = &nfield2; + pnfield3 = &nfield3; + pnfield4 = &nfield4; ptr_id(NULL); - ptr_id(pv1); - ptr_id(pv2); - ptr_id(pv3); - ptr_id(pv4); + ptr_id(pnfield1); + ptr_id(pnfield2); + ptr_id(pnfield3); + ptr_id(pnfield4); /* * With an empty shadow variable hash table, expect not to find * any matches. */ - sv = shadow_get(obj, id); + sv = shadow_get(&obj1, SV_ID1); if (!sv) pr_info(" got expected NULL result\n"); /* * Allocate a few shadow variables with different and . */ - sv1 = shadow_alloc(obj, id, sizeof(pv1), gfp_flags, shadow_ctor, &pv1); + sv1 = shadow_alloc(&obj1, SV_ID1, sizeof(pnfield1), GFP_KERNEL, shadow_ctor, &pnfield1); if (!sv1) return -ENOMEM; - sv2 = shadow_alloc(obj + 1, id, sizeof(pv2), gfp_flags, shadow_ctor, &pv2); + sv2 = shadow_alloc(&obj2, SV_ID1, sizeof(pnfield2), GFP_KERNEL, shadow_ctor, &pnfield2); if (!sv2) return -ENOMEM; - sv3 = shadow_alloc(obj, id + 1, sizeof(pv3), gfp_flags, shadow_ctor, &pv3); + sv3 = shadow_alloc(&obj1, SV_ID2, sizeof(pnfield3), GFP_KERNEL, shadow_ctor, &pnfield3); if (!sv3) return -ENOMEM; @@ -188,23 +200,24 @@ static int test_klp_shadow_vars_init(void) * Verify we can find our new shadow variables and that they point * to expected data. */ - sv = shadow_get(obj, id); + sv = shadow_get(&obj1, SV_ID1); if (!sv) return -EINVAL; - if (sv == sv1 && *sv1 == pv1) + if ((char **)sv == sv1 && *sv1 == pnfield1) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv1), ptr_id(*sv1)); - sv = shadow_get(obj + 1, id); + sv = shadow_get(&obj2, SV_ID1); if (!sv) return -EINVAL; - if (sv == sv2 && *sv2 == pv2) + if ((char **)sv == sv2 && *sv2 == pnfield2) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv2), ptr_id(*sv2)); - sv = shadow_get(obj, id + 1); + + sv = shadow_get(&obj1, SV_ID2); if (!sv) return -EINVAL; - if (sv == sv3 && *sv3 == pv3) + if ((int **)sv == sv3 && *sv3 == pnfield3) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv3), ptr_id(*sv3)); @@ -212,14 +225,14 @@ static int test_klp_shadow_vars_init(void) * Allocate or get a few more, this time with the same , . * The second invocation should return the same shadow var. */ - sv4 = shadow_get_or_alloc(obj + 2, id, sizeof(pv4), gfp_flags, shadow_ctor, &pv4); + sv4 = shadow_get_or_alloc(&obj3, SV_ID1, sizeof(pnfield4), GFP_KERNEL, shadow_ctor, &pnfield4); if (!sv4) return -ENOMEM; - sv = shadow_get_or_alloc(obj + 2, id, sizeof(pv4), gfp_flags, shadow_ctor, &pv4); + sv = shadow_get_or_alloc(&obj3, SV_ID1, sizeof(pnfield4), GFP_KERNEL, shadow_ctor, &pnfield4); if (!sv) return -EINVAL; - if (sv == sv4 && *sv4 == pv4) + if ((int **)sv == sv4 && *sv4 == pnfield4) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv4), ptr_id(*sv4)); @@ -227,36 +240,36 @@ static int test_klp_shadow_vars_init(void) * Free the shadow variables and check that we can no * longer find them. */ - shadow_free(obj, id, shadow_dtor); /* sv1 */ - sv = shadow_get(obj, id); + shadow_free(&obj1, SV_ID1, shadow_dtor); /* sv1 */ + sv = shadow_get(&obj1, SV_ID1); if (!sv) pr_info(" got expected NULL result\n"); - shadow_free(obj + 1, id, shadow_dtor); /* sv2 */ - sv = shadow_get(obj + 1, id); + shadow_free(&obj2, SV_ID1, shadow_dtor); /* sv2 */ + sv = shadow_get(&obj2, SV_ID1); if (!sv) pr_info(" got expected NULL result\n"); - shadow_free(obj + 2, id, shadow_dtor); /* sv4 */ - sv = shadow_get(obj + 2, id); + shadow_free(&obj3, SV_ID1, shadow_dtor); /* sv4 */ + sv = shadow_get(&obj3, SV_ID1); if (!sv) pr_info(" got expected NULL result\n"); /* * We should still find an variable. */ - sv = shadow_get(obj, id + 1); + sv = shadow_get(&obj1, SV_ID2); if (!sv) return -EINVAL; - if (sv == sv3 && *sv3 == pv3) + if ((int **)sv == sv3 && *sv3 == pnfield3) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv3), ptr_id(*sv3)); /* * Free all the variables, too. */ - shadow_free_all(id + 1, shadow_dtor); /* sv3 */ - sv = shadow_get(obj, id); + shadow_free_all(SV_ID2, shadow_dtor); /* sv3 */ + sv = shadow_get(&obj1, SV_ID1); if (!sv) pr_info(" shadow_get() got expected NULL result\n"); From 76efe6da89d8320e9ba65cebe0b3bcb6e5c29b31 Mon Sep 17 00:00:00 2001 From: Yannick Cote Date: Wed, 3 Jun 2020 14:20:57 -0400 Subject: [PATCH 3/9] selftests/livepatch: more verification in test-klp-shadow-vars This change makes the test feel more familiar with narrowing to a typical usage by operating on a number of identical structure instances and populating the same two new shadow variables symmetrically while keeping the same testing and verification criteria for the extra variables. Signed-off-by: Yannick Cote Reviewed-by: Kamalesh Babulal Reviewed-by: Petr Mladek Acked-by: Miroslav Benes Acked-by: Joe Lawrence Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200603182058.109470-4-ycote@redhat.com --- lib/livepatch/test_klp_shadow_vars.c | 184 +++++++++--------- .../selftests/livepatch/test-shadow-vars.sh | 81 +++++--- 2 files changed, 139 insertions(+), 126 deletions(-) diff --git a/lib/livepatch/test_klp_shadow_vars.c b/lib/livepatch/test_klp_shadow_vars.c index ec2635cff974..a49265e56917 100644 --- a/lib/livepatch/test_klp_shadow_vars.c +++ b/lib/livepatch/test_klp_shadow_vars.c @@ -128,6 +128,11 @@ static int shadow_ctor(void *obj, void *shadow_data, void *ctor_data) return 0; } +/* + * With more than one item to free in the list, order is not determined and + * shadow_dtor will not be passed to shadow_free_all() which would make the + * test fail. (see pass 6) + */ static void shadow_dtor(void *obj, void *shadow_data) { int **sv = shadow_data; @@ -136,6 +141,9 @@ static void shadow_dtor(void *obj, void *shadow_data) __func__, ptr_id(obj), ptr_id(sv)); } +/* number of objects we simulate that need shadow vars */ +#define NUM_OBJS 3 + /* dynamically created obj fields have the following shadow var id values */ #define SV_ID1 0x1234 #define SV_ID2 0x1235 @@ -157,122 +165,106 @@ struct test_object { static int test_klp_shadow_vars_init(void) { - struct test_object obj1, obj2, obj3; - char nfield1, nfield2, *pnfield1, *pnfield2, **sv1, **sv2; - int nfield3, nfield4, *pnfield3, *pnfield4, **sv3, **sv4; + struct test_object objs[NUM_OBJS]; + char nfields1[NUM_OBJS], *pnfields1[NUM_OBJS], **sv1[NUM_OBJS]; + char *pndup[NUM_OBJS]; + int nfields2[NUM_OBJS], *pnfields2[NUM_OBJS], **sv2[NUM_OBJS]; void **sv; - - pnfield1 = &nfield1; - pnfield2 = &nfield2; - pnfield3 = &nfield3; - pnfield4 = &nfield4; + int i; ptr_id(NULL); - ptr_id(pnfield1); - ptr_id(pnfield2); - ptr_id(pnfield3); - ptr_id(pnfield4); /* * With an empty shadow variable hash table, expect not to find * any matches. */ - sv = shadow_get(&obj1, SV_ID1); + sv = shadow_get(&objs[0], SV_ID1); if (!sv) pr_info(" got expected NULL result\n"); - /* - * Allocate a few shadow variables with different and . - */ - sv1 = shadow_alloc(&obj1, SV_ID1, sizeof(pnfield1), GFP_KERNEL, shadow_ctor, &pnfield1); - if (!sv1) - return -ENOMEM; + /* pass 1: init & alloc a char+int pair of svars for each objs */ + for (i = 0; i < NUM_OBJS; i++) { + pnfields1[i] = &nfields1[i]; + ptr_id(pnfields1[i]); - sv2 = shadow_alloc(&obj2, SV_ID1, sizeof(pnfield2), GFP_KERNEL, shadow_ctor, &pnfield2); - if (!sv2) - return -ENOMEM; + if (i % 2) { + sv1[i] = shadow_alloc(&objs[i], SV_ID1, + sizeof(pnfields1[i]), GFP_KERNEL, + shadow_ctor, &pnfields1[i]); + } else { + sv1[i] = shadow_get_or_alloc(&objs[i], SV_ID1, + sizeof(pnfields1[i]), GFP_KERNEL, + shadow_ctor, &pnfields1[i]); + } + if (!sv1[i]) + return -ENOMEM; - sv3 = shadow_alloc(&obj1, SV_ID2, sizeof(pnfield3), GFP_KERNEL, shadow_ctor, &pnfield3); - if (!sv3) - return -ENOMEM; + pnfields2[i] = &nfields2[i]; + ptr_id(pnfields2[i]); + sv2[i] = shadow_alloc(&objs[i], SV_ID2, sizeof(pnfields2[i]), + GFP_KERNEL, shadow_ctor, &pnfields2[i]); + if (!sv2[i]) + return -ENOMEM; + } - /* - * Verify we can find our new shadow variables and that they point - * to expected data. - */ - sv = shadow_get(&obj1, SV_ID1); - if (!sv) - return -EINVAL; - if ((char **)sv == sv1 && *sv1 == pnfield1) - pr_info(" got expected PTR%d -> PTR%d result\n", - ptr_id(sv1), ptr_id(*sv1)); + /* pass 2: verify we find allocated svars and where they point to */ + for (i = 0; i < NUM_OBJS; i++) { + /* check the "char" svar for all objects */ + sv = shadow_get(&objs[i], SV_ID1); + if (!sv) + return -EINVAL; + if ((char **)sv == sv1[i] && *sv1[i] == pnfields1[i]) + pr_info(" got expected PTR%d -> PTR%d result\n", + ptr_id(sv1[i]), ptr_id(*sv1[i])); - sv = shadow_get(&obj2, SV_ID1); - if (!sv) - return -EINVAL; - if ((char **)sv == sv2 && *sv2 == pnfield2) - pr_info(" got expected PTR%d -> PTR%d result\n", - ptr_id(sv2), ptr_id(*sv2)); + /* check the "int" svar for all objects */ + sv = shadow_get(&objs[i], SV_ID2); + if (!sv) + return -EINVAL; + if ((int **)sv == sv2[i] && *sv2[i] == pnfields2[i]) + pr_info(" got expected PTR%d -> PTR%d result\n", + ptr_id(sv2[i]), ptr_id(*sv2[i])); + } - sv = shadow_get(&obj1, SV_ID2); - if (!sv) - return -EINVAL; - if ((int **)sv == sv3 && *sv3 == pnfield3) - pr_info(" got expected PTR%d -> PTR%d result\n", - ptr_id(sv3), ptr_id(*sv3)); + /* pass 3: verify that 'get_or_alloc' returns already allocated svars */ + for (i = 0; i < NUM_OBJS; i++) { + pndup[i] = &nfields1[i]; + ptr_id(pndup[i]); - /* - * Allocate or get a few more, this time with the same , . - * The second invocation should return the same shadow var. - */ - sv4 = shadow_get_or_alloc(&obj3, SV_ID1, sizeof(pnfield4), GFP_KERNEL, shadow_ctor, &pnfield4); - if (!sv4) - return -ENOMEM; + sv = shadow_get_or_alloc(&objs[i], SV_ID1, sizeof(pndup[i]), + GFP_KERNEL, shadow_ctor, &pndup[i]); + if (!sv) + return -EINVAL; + if ((char **)sv == sv1[i] && *sv1[i] == pnfields1[i]) + pr_info(" got expected PTR%d -> PTR%d result\n", + ptr_id(sv1[i]), ptr_id(*sv1[i])); + } - sv = shadow_get_or_alloc(&obj3, SV_ID1, sizeof(pnfield4), GFP_KERNEL, shadow_ctor, &pnfield4); - if (!sv) - return -EINVAL; - if ((int **)sv == sv4 && *sv4 == pnfield4) - pr_info(" got expected PTR%d -> PTR%d result\n", - ptr_id(sv4), ptr_id(*sv4)); + /* pass 4: free pairs of svars, verify removal */ + for (i = 0; i < NUM_OBJS; i++) { + shadow_free(&objs[i], SV_ID1, shadow_dtor); /* 'char' pairs */ + sv = shadow_get(&objs[i], SV_ID1); + if (!sv) + pr_info(" got expected NULL result\n"); + } - /* - * Free the shadow variables and check that we can no - * longer find them. - */ - shadow_free(&obj1, SV_ID1, shadow_dtor); /* sv1 */ - sv = shadow_get(&obj1, SV_ID1); - if (!sv) - pr_info(" got expected NULL result\n"); - - shadow_free(&obj2, SV_ID1, shadow_dtor); /* sv2 */ - sv = shadow_get(&obj2, SV_ID1); - if (!sv) - pr_info(" got expected NULL result\n"); - - shadow_free(&obj3, SV_ID1, shadow_dtor); /* sv4 */ - sv = shadow_get(&obj3, SV_ID1); - if (!sv) - pr_info(" got expected NULL result\n"); - - /* - * We should still find an variable. - */ - sv = shadow_get(&obj1, SV_ID2); - if (!sv) - return -EINVAL; - if ((int **)sv == sv3 && *sv3 == pnfield3) - pr_info(" got expected PTR%d -> PTR%d result\n", - ptr_id(sv3), ptr_id(*sv3)); - - /* - * Free all the variables, too. - */ - shadow_free_all(SV_ID2, shadow_dtor); /* sv3 */ - sv = shadow_get(&obj1, SV_ID1); - if (!sv) - pr_info(" shadow_get() got expected NULL result\n"); + /* pass 5: check we still find svar pairs */ + for (i = 0; i < NUM_OBJS; i++) { + sv = shadow_get(&objs[i], SV_ID2); /* 'int' pairs */ + if (!sv) + return -EINVAL; + if ((int **)sv == sv2[i] && *sv2[i] == pnfields2[i]) + pr_info(" got expected PTR%d -> PTR%d result\n", + ptr_id(sv2[i]), ptr_id(*sv2[i])); + } + /* pass 6: free all the svar pairs too. */ + shadow_free_all(SV_ID2, NULL); /* 'int' pairs */ + for (i = 0; i < NUM_OBJS; i++) { + sv = shadow_get(&objs[i], SV_ID2); + if (!sv) + pr_info(" got expected NULL result\n"); + } free_ptr_list(); diff --git a/tools/testing/selftests/livepatch/test-shadow-vars.sh b/tools/testing/selftests/livepatch/test-shadow-vars.sh index 1aae73299114..7c016548c2ea 100755 --- a/tools/testing/selftests/livepatch/test-shadow-vars.sh +++ b/tools/testing/selftests/livepatch/test-shadow-vars.sh @@ -19,42 +19,63 @@ load_mod $MOD_TEST unload_mod $MOD_TEST check_result "% modprobe $MOD_TEST -$MOD_TEST: klp_shadow_get(obj=PTR5, id=0x1234) = PTR0 +$MOD_TEST: klp_shadow_get(obj=PTR1, id=0x1234) = PTR0 $MOD_TEST: got expected NULL result -$MOD_TEST: shadow_ctor: PTR6 -> PTR1 -$MOD_TEST: klp_shadow_alloc(obj=PTR5, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR7, ctor_data=PTR1 = PTR6 -$MOD_TEST: shadow_ctor: PTR8 -> PTR2 -$MOD_TEST: klp_shadow_alloc(obj=PTR9, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR7, ctor_data=PTR2 = PTR8 -$MOD_TEST: shadow_ctor: PTR10 -> PTR3 -$MOD_TEST: klp_shadow_alloc(obj=PTR5, id=0x1235, size=8, gfp_flags=GFP_KERNEL), ctor=PTR7, ctor_data=PTR3 = PTR10 -$MOD_TEST: klp_shadow_get(obj=PTR5, id=0x1234) = PTR6 -$MOD_TEST: got expected PTR6 -> PTR1 result +$MOD_TEST: shadow_ctor: PTR3 -> PTR2 +$MOD_TEST: klp_shadow_get_or_alloc(obj=PTR1, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR2 = PTR3 +$MOD_TEST: shadow_ctor: PTR6 -> PTR5 +$MOD_TEST: klp_shadow_alloc(obj=PTR1, id=0x1235, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR5 = PTR6 +$MOD_TEST: shadow_ctor: PTR8 -> PTR7 +$MOD_TEST: klp_shadow_alloc(obj=PTR9, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR7 = PTR8 +$MOD_TEST: shadow_ctor: PTR11 -> PTR10 +$MOD_TEST: klp_shadow_alloc(obj=PTR9, id=0x1235, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR10 = PTR11 +$MOD_TEST: shadow_ctor: PTR13 -> PTR12 +$MOD_TEST: klp_shadow_get_or_alloc(obj=PTR14, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR12 = PTR13 +$MOD_TEST: shadow_ctor: PTR16 -> PTR15 +$MOD_TEST: klp_shadow_alloc(obj=PTR14, id=0x1235, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR15 = PTR16 +$MOD_TEST: klp_shadow_get(obj=PTR1, id=0x1234) = PTR3 +$MOD_TEST: got expected PTR3 -> PTR2 result +$MOD_TEST: klp_shadow_get(obj=PTR1, id=0x1235) = PTR6 +$MOD_TEST: got expected PTR6 -> PTR5 result $MOD_TEST: klp_shadow_get(obj=PTR9, id=0x1234) = PTR8 -$MOD_TEST: got expected PTR8 -> PTR2 result -$MOD_TEST: klp_shadow_get(obj=PTR5, id=0x1235) = PTR10 -$MOD_TEST: got expected PTR10 -> PTR3 result -$MOD_TEST: shadow_ctor: PTR11 -> PTR4 -$MOD_TEST: klp_shadow_get_or_alloc(obj=PTR12, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR7, ctor_data=PTR4 = PTR11 -$MOD_TEST: klp_shadow_get_or_alloc(obj=PTR12, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR7, ctor_data=PTR4 = PTR11 -$MOD_TEST: got expected PTR11 -> PTR4 result -$MOD_TEST: shadow_dtor(obj=PTR5, shadow_data=PTR6) -$MOD_TEST: klp_shadow_free(obj=PTR5, id=0x1234, dtor=PTR13) -$MOD_TEST: klp_shadow_get(obj=PTR5, id=0x1234) = PTR0 +$MOD_TEST: got expected PTR8 -> PTR7 result +$MOD_TEST: klp_shadow_get(obj=PTR9, id=0x1235) = PTR11 +$MOD_TEST: got expected PTR11 -> PTR10 result +$MOD_TEST: klp_shadow_get(obj=PTR14, id=0x1234) = PTR13 +$MOD_TEST: got expected PTR13 -> PTR12 result +$MOD_TEST: klp_shadow_get(obj=PTR14, id=0x1235) = PTR16 +$MOD_TEST: got expected PTR16 -> PTR15 result +$MOD_TEST: klp_shadow_get_or_alloc(obj=PTR1, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR2 = PTR3 +$MOD_TEST: got expected PTR3 -> PTR2 result +$MOD_TEST: klp_shadow_get_or_alloc(obj=PTR9, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR7 = PTR8 +$MOD_TEST: got expected PTR8 -> PTR7 result +$MOD_TEST: klp_shadow_get_or_alloc(obj=PTR14, id=0x1234, size=8, gfp_flags=GFP_KERNEL), ctor=PTR4, ctor_data=PTR12 = PTR13 +$MOD_TEST: got expected PTR13 -> PTR12 result +$MOD_TEST: shadow_dtor(obj=PTR1, shadow_data=PTR3) +$MOD_TEST: klp_shadow_free(obj=PTR1, id=0x1234, dtor=PTR17) +$MOD_TEST: klp_shadow_get(obj=PTR1, id=0x1234) = PTR0 $MOD_TEST: got expected NULL result $MOD_TEST: shadow_dtor(obj=PTR9, shadow_data=PTR8) -$MOD_TEST: klp_shadow_free(obj=PTR9, id=0x1234, dtor=PTR13) +$MOD_TEST: klp_shadow_free(obj=PTR9, id=0x1234, dtor=PTR17) $MOD_TEST: klp_shadow_get(obj=PTR9, id=0x1234) = PTR0 $MOD_TEST: got expected NULL result -$MOD_TEST: shadow_dtor(obj=PTR12, shadow_data=PTR11) -$MOD_TEST: klp_shadow_free(obj=PTR12, id=0x1234, dtor=PTR13) -$MOD_TEST: klp_shadow_get(obj=PTR12, id=0x1234) = PTR0 +$MOD_TEST: shadow_dtor(obj=PTR14, shadow_data=PTR13) +$MOD_TEST: klp_shadow_free(obj=PTR14, id=0x1234, dtor=PTR17) +$MOD_TEST: klp_shadow_get(obj=PTR14, id=0x1234) = PTR0 $MOD_TEST: got expected NULL result -$MOD_TEST: klp_shadow_get(obj=PTR5, id=0x1235) = PTR10 -$MOD_TEST: got expected PTR10 -> PTR3 result -$MOD_TEST: shadow_dtor(obj=PTR5, shadow_data=PTR10) -$MOD_TEST: klp_shadow_free_all(id=0x1235, dtor=PTR13) -$MOD_TEST: klp_shadow_get(obj=PTR5, id=0x1234) = PTR0 -$MOD_TEST: shadow_get() got expected NULL result -% rmmod test_klp_shadow_vars" +$MOD_TEST: klp_shadow_get(obj=PTR1, id=0x1235) = PTR6 +$MOD_TEST: got expected PTR6 -> PTR5 result +$MOD_TEST: klp_shadow_get(obj=PTR9, id=0x1235) = PTR11 +$MOD_TEST: got expected PTR11 -> PTR10 result +$MOD_TEST: klp_shadow_get(obj=PTR14, id=0x1235) = PTR16 +$MOD_TEST: got expected PTR16 -> PTR15 result +$MOD_TEST: klp_shadow_free_all(id=0x1235, dtor=PTR0) +$MOD_TEST: klp_shadow_get(obj=PTR1, id=0x1235) = PTR0 +$MOD_TEST: got expected NULL result +$MOD_TEST: klp_shadow_get(obj=PTR9, id=0x1235) = PTR0 +$MOD_TEST: got expected NULL result +$MOD_TEST: klp_shadow_get(obj=PTR14, id=0x1235) = PTR0 +$MOD_TEST: got expected NULL result +% rmmod $MOD_TEST" exit 0 From 270f7806d3b91b9c71fa8fe66f7dcc2d6587694e Mon Sep 17 00:00:00 2001 From: Yannick Cote Date: Wed, 3 Jun 2020 14:20:58 -0400 Subject: [PATCH 4/9] selftests/livepatch: fix mem leaks in test-klp-shadow-vars In some cases, when an error occurs during testing and the main test routine returns, a memory leak occurs via leaving previously registered shadow variables allocated in the kernel as well as shadow_ptr list elements. From now on, in case of error, remove all allocated shadow variables and shadow_ptr struct elements. Signed-off-by: Yannick Cote Reviewed-by: Petr Mladek Reviewed-by: Kamalesh Babulal Acked-by: Miroslav Benes Acked-by: Joe Lawrence Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200603182058.109470-5-ycote@redhat.com --- lib/livepatch/test_klp_shadow_vars.c | 43 ++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/lib/livepatch/test_klp_shadow_vars.c b/lib/livepatch/test_klp_shadow_vars.c index a49265e56917..b99116490858 100644 --- a/lib/livepatch/test_klp_shadow_vars.c +++ b/lib/livepatch/test_klp_shadow_vars.c @@ -170,6 +170,7 @@ static int test_klp_shadow_vars_init(void) char *pndup[NUM_OBJS]; int nfields2[NUM_OBJS], *pnfields2[NUM_OBJS], **sv2[NUM_OBJS]; void **sv; + int ret; int i; ptr_id(NULL); @@ -196,31 +197,39 @@ static int test_klp_shadow_vars_init(void) sizeof(pnfields1[i]), GFP_KERNEL, shadow_ctor, &pnfields1[i]); } - if (!sv1[i]) - return -ENOMEM; + if (!sv1[i]) { + ret = -ENOMEM; + goto out; + } pnfields2[i] = &nfields2[i]; ptr_id(pnfields2[i]); sv2[i] = shadow_alloc(&objs[i], SV_ID2, sizeof(pnfields2[i]), GFP_KERNEL, shadow_ctor, &pnfields2[i]); - if (!sv2[i]) - return -ENOMEM; + if (!sv2[i]) { + ret = -ENOMEM; + goto out; + } } /* pass 2: verify we find allocated svars and where they point to */ for (i = 0; i < NUM_OBJS; i++) { /* check the "char" svar for all objects */ sv = shadow_get(&objs[i], SV_ID1); - if (!sv) - return -EINVAL; + if (!sv) { + ret = -EINVAL; + goto out; + } if ((char **)sv == sv1[i] && *sv1[i] == pnfields1[i]) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv1[i]), ptr_id(*sv1[i])); /* check the "int" svar for all objects */ sv = shadow_get(&objs[i], SV_ID2); - if (!sv) - return -EINVAL; + if (!sv) { + ret = -EINVAL; + goto out; + } if ((int **)sv == sv2[i] && *sv2[i] == pnfields2[i]) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv2[i]), ptr_id(*sv2[i])); @@ -233,8 +242,10 @@ static int test_klp_shadow_vars_init(void) sv = shadow_get_or_alloc(&objs[i], SV_ID1, sizeof(pndup[i]), GFP_KERNEL, shadow_ctor, &pndup[i]); - if (!sv) - return -EINVAL; + if (!sv) { + ret = -EINVAL; + goto out; + } if ((char **)sv == sv1[i] && *sv1[i] == pnfields1[i]) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv1[i]), ptr_id(*sv1[i])); @@ -251,8 +262,10 @@ static int test_klp_shadow_vars_init(void) /* pass 5: check we still find svar pairs */ for (i = 0; i < NUM_OBJS; i++) { sv = shadow_get(&objs[i], SV_ID2); /* 'int' pairs */ - if (!sv) - return -EINVAL; + if (!sv) { + ret = -EINVAL; + goto out; + } if ((int **)sv == sv2[i] && *sv2[i] == pnfields2[i]) pr_info(" got expected PTR%d -> PTR%d result\n", ptr_id(sv2[i]), ptr_id(*sv2[i])); @@ -269,6 +282,12 @@ static int test_klp_shadow_vars_init(void) free_ptr_list(); return 0; +out: + shadow_free_all(SV_ID1, NULL); /* 'char' pairs */ + shadow_free_all(SV_ID2, NULL); /* 'int' pairs */ + free_ptr_list(); + + return ret; } static void test_klp_shadow_vars_exit(void) From 2eeb0d457d13ed7b170e13cd99d15509d0dd832d Mon Sep 17 00:00:00 2001 From: Joe Lawrence Date: Thu, 18 Jun 2020 14:10:38 -0400 Subject: [PATCH 5/9] selftests/livepatch: Don't clear dmesg when running tests Inspired by commit f131d9edc29d ("selftests/lkdtm: Don't clear dmesg when running tests"), keep a reference dmesg copy when beginning each test. This way check_result() can compare against the initial copy rather than relying upon an empty log. Signed-off-by: Joe Lawrence Reviewed-by: Kamalesh Babulal Reviewed-by: Yannick Cote Reviewed-by: Petr Mladek Acked-by: Miroslav Benes Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200618181040.21132-2-joe.lawrence@redhat.com --- tools/testing/selftests/livepatch/README | 16 +++--- .../testing/selftests/livepatch/functions.sh | 35 +++++++++++- .../selftests/livepatch/test-callbacks.sh | 55 ++++--------------- .../selftests/livepatch/test-ftrace.sh | 4 +- .../selftests/livepatch/test-livepatch.sh | 12 +--- .../selftests/livepatch/test-shadow-vars.sh | 4 +- .../testing/selftests/livepatch/test-state.sh | 21 +++---- 7 files changed, 66 insertions(+), 81 deletions(-) diff --git a/tools/testing/selftests/livepatch/README b/tools/testing/selftests/livepatch/README index 621d325425c2..0942dd5826f8 100644 --- a/tools/testing/selftests/livepatch/README +++ b/tools/testing/selftests/livepatch/README @@ -6,8 +6,8 @@ This is a small set of sanity tests for the kernel livepatching. The test suite loads and unloads several test kernel modules to verify livepatch behavior. Debug information is logged to the kernel's message -buffer and parsed for expected messages. (Note: the tests will clear -the message buffer between individual tests.) +buffer and parsed for expected messages. (Note: the tests will compare +the message buffer for only the duration of each individual test.) Config @@ -35,9 +35,9 @@ Adding tests ------------ See the common functions.sh file for the existing collection of utility -functions, most importantly setup_config() and check_result(). The -latter function greps the kernel's ring buffer for "livepatch:" and -"test_klp" strings, so tests be sure to include one of those strings for -result comparison. Other utility functions include general module -loading and livepatch loading helpers (waiting for patch transitions, -sysfs entries, etc.) +functions, most importantly setup_config(), start_test() and +check_result(). The latter function greps the kernel's ring buffer for +"livepatch:" and "test_klp" strings, so tests be sure to include one of +those strings for result comparison. Other utility functions include +general module loading and livepatch loading helpers (waiting for patch +transitions, sysfs entries, etc.) diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh index 2aab9791791d..bf73ec4f4a71 100644 --- a/tools/testing/selftests/livepatch/functions.sh +++ b/tools/testing/selftests/livepatch/functions.sh @@ -41,6 +41,17 @@ function die() { exit 1 } +# save existing dmesg so we can detect new content +function save_dmesg() { + SAVED_DMESG=$(mktemp --tmpdir -t klp-dmesg-XXXXXX) + dmesg > "$SAVED_DMESG" +} + +# cleanup temporary dmesg file from save_dmesg() +function cleanup_dmesg_file() { + rm -f "$SAVED_DMESG" +} + function push_config() { DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \ awk -F'[: ]' '{print "file " $1 " line " $2 " " $4}') @@ -68,6 +79,11 @@ function set_ftrace_enabled() { echo "livepatch: $result" > /dev/kmsg } +function cleanup() { + pop_config + cleanup_dmesg_file +} + # setup_config - save the current config and set a script exit trap that # restores the original config. Setup the dynamic debug # for verbose livepatching output and turn on @@ -77,7 +93,7 @@ function setup_config() { push_config set_dynamic_debug set_ftrace_enabled 1 - trap pop_config EXIT INT TERM HUP + trap cleanup EXIT INT TERM HUP } # loop_until(cmd) - loop a command until it is successful or $MAX_RETRIES, @@ -243,13 +259,26 @@ function set_pre_patch_ret { die "failed to set pre_patch_ret parameter for $mod module" } +function start_test { + local test="$1" + + save_dmesg + echo -n "TEST: $test ... " +} + # check_result() - verify dmesg output # TODO - better filter, out of order msgs, etc? function check_result { local expect="$*" local result - result=$(dmesg | grep -v 'tainting' | grep -e 'livepatch:' -e 'test_klp' | sed 's/^\[[ 0-9.]*\] //') + # Note: when comparing dmesg output, the kernel log timestamps + # help differentiate repeated testing runs. Remove them with a + # post-comparison sed filter. + + result=$(dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$SAVED_DMESG" - | \ + grep -v 'tainting' | grep -e 'livepatch:' -e 'test_klp' | \ + sed 's/^\[[ 0-9.]*\] //') if [[ "$expect" == "$result" ]] ; then echo "ok" @@ -257,4 +286,6 @@ function check_result { echo -e "not ok\n\n$(diff -upr --label expected --label result <(echo "$expect") <(echo "$result"))\n" die "livepatch kselftest(s) failed" fi + + cleanup_dmesg_file } diff --git a/tools/testing/selftests/livepatch/test-callbacks.sh b/tools/testing/selftests/livepatch/test-callbacks.sh index 32b57ba07f4f..90b26dbb2626 100755 --- a/tools/testing/selftests/livepatch/test-callbacks.sh +++ b/tools/testing/selftests/livepatch/test-callbacks.sh @@ -12,8 +12,6 @@ MOD_TARGET_BUSY=test_klp_callbacks_busy setup_config -# TEST: target module before livepatch -# # Test a combination of loading a kernel module and a livepatch that # patches a function in the first module. Load the target module # before the livepatch module. Unload them in the same order. @@ -28,8 +26,7 @@ setup_config # unpatching transition starts. klp_objects are reverted, post-patch # callbacks execute and the transition completes. -echo -n "TEST: target module before livepatch ... " -dmesg -C +start_test "target module before livepatch" load_mod $MOD_TARGET load_lp $MOD_LIVEPATCH @@ -63,8 +60,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete $MOD_TARGET: ${MOD_TARGET}_exit" -# TEST: module_coming notifier -# # This test is similar to the previous test, but (un)load the livepatch # module before the target kernel module. This tests the livepatch # core's module_coming handler. @@ -78,8 +73,7 @@ $MOD_TARGET: ${MOD_TARGET}_exit" # - On livepatch disable, all currently loaded klp_objects' (vmlinux and # $MOD_TARGET) pre/post-unpatch callbacks are executed. -echo -n "TEST: module_coming notifier ... " -dmesg -C +start_test "module_coming notifier" load_lp $MOD_LIVEPATCH load_mod $MOD_TARGET @@ -114,8 +108,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete $MOD_TARGET: ${MOD_TARGET}_exit" -# TEST: module_going notifier -# # Test loading the livepatch after a targeted kernel module, then unload # the kernel module before disabling the livepatch. This tests the # livepatch core's module_going handler. @@ -129,8 +121,7 @@ $MOD_TARGET: ${MOD_TARGET}_exit" # - When the livepatch is disabled, pre and post-unpatch callbacks are # run for the remaining klp_object, vmlinux. -echo -n "TEST: module_going notifier ... " -dmesg -C +start_test "module_going notifier" load_mod $MOD_TARGET load_lp $MOD_LIVEPATCH @@ -165,8 +156,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: module_coming and module_going notifiers -# # This test is similar to the previous test, however the livepatch is # loaded first. This tests the livepatch core's module_coming and # module_going handlers. @@ -180,8 +169,7 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete # from the $MOD_TARGET klp_object. As such, only pre and # post-unpatch callbacks are executed when this occurs. -echo -n "TEST: module_coming and module_going notifiers ... " -dmesg -C +start_test "module_coming and module_going notifiers" load_lp $MOD_LIVEPATCH load_mod $MOD_TARGET @@ -217,8 +205,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: target module not present -# # A simple test of loading a livepatch without one of its patch target # klp_objects ever loaded ($MOD_TARGET). # @@ -227,8 +213,7 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete # - As expected, only pre/post-(un)patch handlers are executed for # vmlinux. -echo -n "TEST: target module not present ... " -dmesg -C +start_test "target module not present" load_lp $MOD_LIVEPATCH disable_lp $MOD_LIVEPATCH @@ -252,8 +237,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: pre-patch callback -ENODEV -# # Test a scenario where a vmlinux pre-patch callback returns a non-zero # status (ie, failure). # @@ -265,8 +248,7 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete # The result is that the insmod command refuses to load the livepatch # module. -echo -n "TEST: pre-patch callback -ENODEV ... " -dmesg -C +start_test "pre-patch callback -ENODEV" load_mod $MOD_TARGET load_failing_mod $MOD_LIVEPATCH pre_patch_ret=-19 @@ -288,8 +270,6 @@ modprobe: ERROR: could not insert '$MOD_LIVEPATCH': No such device $MOD_TARGET: ${MOD_TARGET}_exit" -# TEST: module_coming + pre-patch callback -ENODEV -# # Similar to the previous test, setup a livepatch such that its vmlinux # pre-patch callback returns success. However, when a targeted kernel # module is later loaded, have the livepatch return a failing status @@ -307,8 +287,7 @@ $MOD_TARGET: ${MOD_TARGET}_exit" # # - Pre/post-unpatch callbacks are run for the vmlinux klp_object. -echo -n "TEST: module_coming + pre-patch callback -ENODEV ... " -dmesg -C +start_test "module_coming + pre-patch callback -ENODEV" load_lp $MOD_LIVEPATCH set_pre_patch_ret $MOD_LIVEPATCH -19 @@ -341,8 +320,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: multiple target modules -# # Test loading multiple targeted kernel modules. This test-case is # mainly for comparing with the next test-case. # @@ -353,8 +330,7 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete # module. Post-patch callbacks are executed and the transition # completes quickly. -echo -n "TEST: multiple target modules ... " -dmesg -C +start_test "multiple target modules" load_mod $MOD_TARGET_BUSY block_transition=N load_lp $MOD_LIVEPATCH @@ -402,8 +378,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit" -# TEST: busy target module -# # A similar test as the previous one, but force the "busy" kernel module # to block the livepatch transition. # @@ -431,8 +405,7 @@ $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit" # klp_object's post-patch callbacks executed, the remaining # klp_object's pre-unpatch callbacks are skipped. -echo -n "TEST: busy target module ... " -dmesg -C +start_test "busy target module" load_mod $MOD_TARGET_BUSY block_transition=Y load_lp_nowait $MOD_LIVEPATCH @@ -478,8 +451,6 @@ $MOD_TARGET_BUSY: busymod_work_func exit $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit" -# TEST: multiple livepatches -# # Test loading multiple livepatches. This test-case is mainly for comparing # with the next test-case. # @@ -487,8 +458,7 @@ $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit" # execute as each patch progresses through its (un)patching # transition. -echo -n "TEST: multiple livepatches ... " -dmesg -C +start_test "multiple livepatches" load_lp $MOD_LIVEPATCH load_lp $MOD_LIVEPATCH2 @@ -531,8 +501,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: atomic replace -# # Load multiple livepatches, but the second as an 'atomic-replace' # patch. When the latter loads, the original livepatch should be # disabled and *none* of its pre/post-unpatch callbacks executed. On @@ -547,8 +515,7 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete # - Once the atomic replace module is loaded, only its pre and post # unpatch callbacks are executed. -echo -n "TEST: atomic replace ... " -dmesg -C +start_test "atomic replace" load_lp $MOD_LIVEPATCH load_lp $MOD_LIVEPATCH2 replace=1 diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh index e2a76887f40a..9160c9ec3b6f 100755 --- a/tools/testing/selftests/livepatch/test-ftrace.sh +++ b/tools/testing/selftests/livepatch/test-ftrace.sh @@ -9,13 +9,11 @@ MOD_LIVEPATCH=test_klp_livepatch setup_config -# TEST: livepatch interaction with ftrace_enabled sysctl # - turn ftrace_enabled OFF and verify livepatches can't load # - turn ftrace_enabled ON and verify livepatch can load # - verify that ftrace_enabled can't be turned OFF while a livepatch is loaded -echo -n "TEST: livepatch interaction with ftrace_enabled sysctl ... " -dmesg -C +start_test "livepatch interaction with ftrace_enabled sysctl" set_ftrace_enabled 0 load_failing_mod $MOD_LIVEPATCH diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh index 493e3df415a1..5fe79ac34be1 100755 --- a/tools/testing/selftests/livepatch/test-livepatch.sh +++ b/tools/testing/selftests/livepatch/test-livepatch.sh @@ -10,13 +10,11 @@ MOD_REPLACE=test_klp_atomic_replace setup_config -# TEST: basic function patching # - load a livepatch that modifies the output from /proc/cmdline and # verify correct behavior # - unload the livepatch and make sure the patch was removed -echo -n "TEST: basic function patching ... " -dmesg -C +start_test "basic function patching" load_lp $MOD_LIVEPATCH @@ -47,15 +45,13 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: multiple livepatches # - load a livepatch that modifies the output from /proc/cmdline and # verify correct behavior # - load another livepatch and verify that both livepatches are active # - unload the second livepatch and verify that the first is still active # - unload the first livepatch and verify none are active -echo -n "TEST: multiple livepatches ... " -dmesg -C +start_test "multiple livepatches" load_lp $MOD_LIVEPATCH @@ -109,7 +105,6 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: atomic replace livepatch # - load a livepatch that modifies the output from /proc/cmdline and # verify correct behavior # - load an atomic replace livepatch and verify that only the second is active @@ -117,8 +112,7 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete # is still active # - remove the atomic replace livepatch and verify that none are active -echo -n "TEST: atomic replace livepatch ... " -dmesg -C +start_test "atomic replace livepatch" load_lp $MOD_LIVEPATCH diff --git a/tools/testing/selftests/livepatch/test-shadow-vars.sh b/tools/testing/selftests/livepatch/test-shadow-vars.sh index 7c016548c2ea..e04cb354f56b 100755 --- a/tools/testing/selftests/livepatch/test-shadow-vars.sh +++ b/tools/testing/selftests/livepatch/test-shadow-vars.sh @@ -9,11 +9,9 @@ MOD_TEST=test_klp_shadow_vars setup_config -# TEST: basic shadow variable API # - load a module that exercises the shadow variable API -echo -n "TEST: basic shadow variable API ... " -dmesg -C +start_test "basic shadow variable API" load_mod $MOD_TEST unload_mod $MOD_TEST diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh index a08212708115..38656721c958 100755 --- a/tools/testing/selftests/livepatch/test-state.sh +++ b/tools/testing/selftests/livepatch/test-state.sh @@ -10,10 +10,10 @@ MOD_LIVEPATCH3=test_klp_state3 setup_config -# TEST: Loading and removing a module that modifies the system state -echo -n "TEST: system state modification ... " -dmesg -C +# Load and remove a module that modifies the system state + +start_test "system state modification" load_lp $MOD_LIVEPATCH disable_lp $MOD_LIVEPATCH @@ -41,10 +41,9 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete % rmmod $MOD_LIVEPATCH" -# TEST: Take over system state change by a cumulative patch +# Take over system state change by a cumulative patch -echo -n "TEST: taking over system state modification ... " -dmesg -C +start_test "taking over system state modification" load_lp $MOD_LIVEPATCH load_lp $MOD_LIVEPATCH2 @@ -85,10 +84,9 @@ livepatch: '$MOD_LIVEPATCH2': unpatching complete % rmmod $MOD_LIVEPATCH2" -# TEST: Take over system state change by a cumulative patch +# Take over system state change by a cumulative patch -echo -n "TEST: compatible cumulative livepatches ... " -dmesg -C +start_test "compatible cumulative livepatches" load_lp $MOD_LIVEPATCH2 load_lp $MOD_LIVEPATCH3 @@ -142,10 +140,9 @@ livepatch: '$MOD_LIVEPATCH2': unpatching complete % rmmod $MOD_LIVEPATCH3" -# TEST: Failure caused by incompatible cumulative livepatches +# Failure caused by incompatible cumulative livepatches -echo -n "TEST: incompatible cumulative livepatches ... " -dmesg -C +start_test "incompatible cumulative livepatches" load_lp $MOD_LIVEPATCH2 load_failing_mod $MOD_LIVEPATCH From c401088f0f1887761b8472352fd4f23595a74149 Mon Sep 17 00:00:00 2001 From: Joe Lawrence Date: Thu, 18 Jun 2020 14:10:39 -0400 Subject: [PATCH 6/9] selftests/livepatch: refine dmesg 'taints' in dmesg comparison The livepatch selftests currently grep on "taints" to filter out "tainting kernel with TAINT_LIVEPATCH" messages which may be logged when loading livepatch modules. Further filter the log to drop "loading out-of-tree module taints kernel" in the rare case the klp_test modules have been built out-of-tree. Look for the longer "taints kernel" or "tainting kernel" strings to avoid inadvertent partial matching. Signed-off-by: Joe Lawrence Reviewed-by: Kamalesh Babulal Reviewed-by: Yannick Cote Reviewed-by: Petr Mladek Acked-by: Miroslav Benes Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200618181040.21132-3-joe.lawrence@redhat.com --- tools/testing/selftests/livepatch/functions.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh index bf73ec4f4a71..5e5a79179fc1 100644 --- a/tools/testing/selftests/livepatch/functions.sh +++ b/tools/testing/selftests/livepatch/functions.sh @@ -277,7 +277,8 @@ function check_result { # post-comparison sed filter. result=$(dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$SAVED_DMESG" - | \ - grep -v 'tainting' | grep -e 'livepatch:' -e 'test_klp' | \ + grep -e 'livepatch:' -e 'test_klp' | \ + grep -v '\(tainting\|taints\) kernel' | \ sed 's/^\[[ 0-9.]*\] //') if [[ "$expect" == "$result" ]] ; then From 3fd9bd8b7e41a1908bf8bc0cd06606f2b787cd39 Mon Sep 17 00:00:00 2001 From: Joe Lawrence Date: Thu, 18 Jun 2020 14:10:40 -0400 Subject: [PATCH 7/9] selftests/livepatch: add test delimiter to dmesg Make it bit easier to parse the kernel logs during the selftests by adding a "===== TEST: $test =====" delimiter when each individual test begins. Suggested-by: Petr Mladek Signed-off-by: Joe Lawrence Reviewed-by: Kamalesh Babulal Reviewed-by: Yannick Cote Reviewed-by: Petr Mladek Acked-by: Miroslav Benes Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200618181040.21132-4-joe.lawrence@redhat.com --- tools/testing/selftests/livepatch/functions.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh index 5e5a79179fc1..36648ca367c2 100644 --- a/tools/testing/selftests/livepatch/functions.sh +++ b/tools/testing/selftests/livepatch/functions.sh @@ -264,6 +264,7 @@ function start_test { save_dmesg echo -n "TEST: $test ... " + log "===== TEST: $test =====" } # check_result() - verify dmesg output From 2f3f651f3756ef9beff704ae4d8d85e797df5b3b Mon Sep 17 00:00:00 2001 From: Joe Lawrence Date: Fri, 10 Jul 2020 14:37:45 -0400 Subject: [PATCH 8/9] selftests/livepatch: Use "comm" instead of "diff" for dmesg BusyBox diff doesn't support the GNU diff '--LTYPE-line-format' options that were used in the selftests to filter older kernel log messages from dmesg output. Use "comm" which is more available in smaller boot environments. Reported-by: Naresh Kamboju Signed-off-by: Joe Lawrence Reviewed-by: Yannick Cote Reviewed-by: Kamalesh Babulal Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200710183745.19730-1-joe.lawrence@redhat.com --- tools/testing/selftests/livepatch/functions.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh index 36648ca367c2..408529d94ddb 100644 --- a/tools/testing/selftests/livepatch/functions.sh +++ b/tools/testing/selftests/livepatch/functions.sh @@ -277,7 +277,7 @@ function check_result { # help differentiate repeated testing runs. Remove them with a # post-comparison sed filter. - result=$(dmesg | diff --changed-group-format='%>' --unchanged-group-format='' "$SAVED_DMESG" - | \ + result=$(dmesg | comm -13 "$SAVED_DMESG" - | \ grep -e 'livepatch:' -e 'test_klp' | \ grep -v '\(tainting\|taints\) kernel' | \ sed 's/^\[[ 0-9.]*\] //') From 5e4d46881f29a93f35f3aefeebc80cebfb44ef71 Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Tue, 14 Jul 2020 11:10:30 +0200 Subject: [PATCH 9/9] selftests/livepatch: adopt to newer sysctl error format With procfs v3.3.16, the sysctl command doesn't print the set key and value on error. This change breaks livepatch selftest test-ftrace.sh, that tests the interaction of sysctl ftrace_enabled: Make it work with all sysctl versions using '-q' option. Explicitly print the final status on success so that it can be verified in the log. The error message is enough on failure. Reported-by: Kamalesh Babulal Signed-off-by: Petr Mladek Reviewed-by: Kamalesh Babulal Reviewed-by: Joe Lawrence Acked-by: Miroslav Benes Link: https://lore.kernel.org/r/20200714091030.1611-1-pmladek@suse.com --- tools/testing/selftests/livepatch/functions.sh | 3 ++- tools/testing/selftests/livepatch/test-ftrace.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh index 408529d94ddb..1aba83c87ad3 100644 --- a/tools/testing/selftests/livepatch/functions.sh +++ b/tools/testing/selftests/livepatch/functions.sh @@ -75,7 +75,8 @@ function set_dynamic_debug() { } function set_ftrace_enabled() { - result=$(sysctl kernel.ftrace_enabled="$1" 2>&1 | paste --serial --delimiters=' ') + result=$(sysctl -q kernel.ftrace_enabled="$1" 2>&1 && \ + sysctl kernel.ftrace_enabled 2>&1) echo "livepatch: $result" > /dev/kmsg } diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh index 9160c9ec3b6f..552e165512f4 100755 --- a/tools/testing/selftests/livepatch/test-ftrace.sh +++ b/tools/testing/selftests/livepatch/test-ftrace.sh @@ -51,7 +51,7 @@ livepatch: '$MOD_LIVEPATCH': initializing patching transition livepatch: '$MOD_LIVEPATCH': starting patching transition livepatch: '$MOD_LIVEPATCH': completing patching transition livepatch: '$MOD_LIVEPATCH': patching complete -livepatch: sysctl: setting key \"kernel.ftrace_enabled\": Device or resource busy kernel.ftrace_enabled = 0 +livepatch: sysctl: setting key \"kernel.ftrace_enabled\": Device or resource busy % echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH/enabled livepatch: '$MOD_LIVEPATCH': initializing unpatching transition livepatch: '$MOD_LIVEPATCH': starting unpatching transition