On Mon, Jul 06, 2020 at 09:13:09PM +0000, Uriel Guajardo wrote:
> From: Uriel Guajardo <urielguajardo@xxxxxxxxxx>
>
> Integrate kmemleak into the KUnit testing framework.
>
> Kmemleak will now fail the currently running KUnit test case if it finds
> any memory leaks.
>
> The minimum object age for reporting is set to 0 msecs so that leaks are
> not ignored if the test case finishes too quickly.
>
> Signed-off-by: Uriel Guajardo <urielguajardo@xxxxxxxxxx>
> ---
> include/linux/kmemleak.h | 11 +++++++++++
> lib/Kconfig.debug | 26 ++++++++++++++++++++++++++
> lib/kunit/test.c | 36 +++++++++++++++++++++++++++++++++++-
> mm/kmemleak.c | 27 +++++++++++++++++++++------
> 4 files changed, 93 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
> index 34684b2026ab..0da427934462 100644
> --- a/include/linux/kmemleak.h
> +++ b/include/linux/kmemleak.h
> @@ -35,6 +35,10 @@ extern void kmemleak_free_part_phys(phys_addr_t phys, size_t size) __ref;
> extern void kmemleak_not_leak_phys(phys_addr_t phys) __ref;
> extern void kmemleak_ignore_phys(phys_addr_t phys) __ref;
>
> +extern ssize_t kmemleak_write(struct file *file,
> + const char __user *user_buf,
> + size_t size, loff_t *ppos);
> +
> static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
> int min_count, slab_flags_t flags,
> gfp_t gfp)
> @@ -120,6 +124,13 @@ static inline void kmemleak_ignore_phys(phys_addr_t phys)
> {
> }
>
> +static inline ssize_t kmemleak_write(struct file *file,
> + const char __user *user_buf,
> + size_t size, loff_t *ppos)
> +{
> + return -1;
> +}
> +
> #endif /* CONFIG_DEBUG_KMEMLEAK */
>
> #endif /* __KMEMLEAK_H */
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 21d9c5f6e7ec..e9c492cb3f4d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -602,6 +602,32 @@ config DEBUG_KMEMLEAK_MEM_POOL_SIZE
> fully initialised, this memory pool acts as an emergency one
> if slab allocations fail.
>
> +config DEBUG_KMEMLEAK_MAX_TRACE
> + int "Kmemleak stack trace length"
> + depends on DEBUG_KMEMLEAK
> + default 16
> +
> +config DEBUG_KMEMLEAK_MSECS_MIN_AGE
> + int "Minimum object age before reporting in msecs"
> + depends on DEBUG_KMEMLEAK
> + default 0 if KUNIT
> + default 5000
> +
> +config DEBUG_KMEMLEAK_SECS_FIRST_SCAN
> + int "Delay before first scan in secs"
> + depends on DEBUG_KMEMLEAK
> + default 60
> +
> +config DEBUG_KMEMLEAK_SECS_SCAN_WAIT
> + int "Delay before subsequent auto scans in secs"
> + depends on DEBUG_KMEMLEAK
> + default 600
> +
> +config DEBUG_KMEMLEAK_MAX_SCAN_SIZE
> + int "Maximum size of scanned block"
> + depends on DEBUG_KMEMLEAK
> + default 4096
> +
Why do you make those configurable? I don't see anywhere you make use of
them except DEBUG_KMEMLEAK_MSECS_MIN_AGE?
Even then, how setting DEBUG_KMEMLEAK_MSECS_MIN_AGE=0 not giving too
many false positives? Kmemleak simply does not work that instantly.
> config DEBUG_KMEMLEAK_TEST
> tristate "Simple test for the kernel memory leak detector"
> depends on DEBUG_KMEMLEAK && m
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 8580ed831a8f..8d113a6a214b 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -11,6 +11,7 @@
> #include <linux/kref.h>
> #include <linux/sched/debug.h>
> #include <linux/sched.h>
> +#include <linux/kmemleak.h>
>
> #include "debugfs.h"
> #include "string-stream.h"
> @@ -277,6 +278,27 @@ static void kunit_run_case_cleanup(struct kunit *test,
> kunit_case_internal_cleanup(test);
> }
>
> +/*
> + * Manually scans for memory leaks using the kmemleak tool.
> + *
> + * Any leaks that occurred since the previous scan will be
> + * reported and will cause the currently running test to fail.
> + */
> +static inline void kmemleak_scan(void)
> +{
> + loff_t pos;
> + kmemleak_write(NULL, "scan", 5, &pos);
> +}
> +
> +/*
> + * Turn off the background automatic scan that kmemleak performs upon starting
> + */
> +static inline void kmemleak_automatic_scan_off(void)
> +{
> + loff_t pos;
> + kmemleak_write(NULL, "scan=off", 9, &pos);
> +}
> +
> struct kunit_try_catch_context {
> struct kunit *test;
> struct kunit_suite *suite;
> @@ -290,6 +312,12 @@ static void kunit_try_run_case(void *data)
> struct kunit_suite *suite = ctx->suite;
> struct kunit_case *test_case = ctx->test_case;
>
> + /*
> + * Clear any reported memory leaks since last scan, so that only the
> + * leaks pertaining to the test case remain afterwards.
> + */
> + kmemleak_scan();
> +
> current->kunit_test = test;
>
> /*
> @@ -298,7 +326,12 @@ static void kunit_try_run_case(void *data)
> * thread will resume control and handle any necessary clean up.
> */
> kunit_run_case_internal(test, suite, test_case);
> - /* This line may never be reached. */
> +
> + /* These lines may never be reached. */
> +
> + /* Report any detected memory leaks that occurred in the test case */
> + kmemleak_scan();
> +
> kunit_run_case_cleanup(test, suite);
> }
>
> @@ -388,6 +421,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
> int __kunit_test_suites_init(struct kunit_suite **suites)
> {
> unsigned int i;
> + kmemleak_automatic_scan_off();
>
> for (i = 0; suites[i] != NULL; i++) {
> kunit_init_suite(suites[i]);
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index e362dc3d2028..ad046c77e00c 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -99,15 +99,26 @@
> #include <linux/kasan.h>
> #include <linux/kmemleak.h>
> #include <linux/memory_hotplug.h>
> +#include <kunit/test.h>
>
> /*
> * Kmemleak configuration and common defines.
> */
> -#define MAX_TRACE 16 /* stack trace length */
> -#define MSECS_MIN_AGE 5000 /* minimum object age for reporting */
> -#define SECS_FIRST_SCAN 60 /* delay before the first scan */
> -#define SECS_SCAN_WAIT 600 /* subsequent auto scanning delay */
> -#define MAX_SCAN_SIZE 4096 /* maximum size of a scanned block */
> +
> +/* stack trace length */
> +#define MAX_TRACE CONFIG_DEBUG_KMEMLEAK_MAX_TRACE
> +
> +/* minimum object age for reporting */
> +#define MSECS_MIN_AGE CONFIG_DEBUG_KMEMLEAK_MSECS_MIN_AGE
> +
> +/* delay before the first scan */
> +#define SECS_FIRST_SCAN CONFIG_DEBUG_KMEMLEAK_SECS_FIRST_SCAN
> +
> +/* subsequent auto scanning delay */
> +#define SECS_SCAN_WAIT CONFIG_DEBUG_KMEMLEAK_SECS_SCAN_WAIT
> +
> +/* maximum size of a scanned lock */
> +#define MAX_SCAN_SIZE CONFIG_DEBUG_KMEMLEAK_MAX_SCAN_SIZE
>
> #define BYTES_PER_POINTER sizeof(void *)
>
> @@ -1490,6 +1501,7 @@ static void kmemleak_scan(void)
> * Check for new or unreferenced objects modified since the previous
> * scan and color them gray until the next scan.
> */
> +#if (!IS_ENABLED(CONFIG_KUNIT))
> rcu_read_lock();
> list_for_each_entry_rcu(object, &object_list, object_list) {
> raw_spin_lock_irqsave(&object->lock, flags);
> @@ -1502,6 +1514,7 @@ static void kmemleak_scan(void)
> raw_spin_unlock_irqrestore(&object->lock, flags);
> }
> rcu_read_unlock();
> +#endif
>
> /*
> * Re-scan the gray list for modified unreferenced objects.
> @@ -1534,6 +1547,8 @@ static void kmemleak_scan(void)
> rcu_read_unlock();
>
> if (new_leaks) {
> + kunit_fail_current_test();
> +
> kmemleak_found_leaks = true;
>
> pr_info("%d new suspected memory leaks (see /sys/kernel/debug/kmemleak)\n",
> @@ -1764,7 +1779,7 @@ static void __kmemleak_do_cleanup(void);
> * if kmemleak has been disabled.
> * dump=... - dump information about the object found at the given address
> */
> -static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
> +ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
> size_t size, loff_t *ppos)
> {
> char buf[64];
> --
> 2.27.0.212.ge8ba1cc988-goog
>
>
