2023-11-16 22:43:45

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 0/2] tweak kmemleak report format

These 2 patches make minor changes to the report:

1st strips "age <increasing>" from output. This makes the output
idempotent; unchanging until a new leak is reported.

2nd adds the backtrace.checksum to the "backtrace:" line. This lets a
user see repeats without actually reading the whole backtrace. So now
the backtrace line looks like this:

backtrace (ck 603070071):

I surveyed for un-wanted effects upon users:

Syzkaller parses kmemleak in executor/common_linux.h:
static void check_leaks(char** frames, int nframes)

It just counts occurrences of "unreferenced object", specifically it
does not look for "age", nor would it choke on "crc" being added.

github has 3 repos with "kmemleak" mentioned, all are moribund.
gitlab has 0 hits on "kmemleak".


Jim Cromie (2):
kmemleak: drop (age <increasing>) from leak record
kmemleak: add checksum to backtrace report

mm/kmemleak.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

--
2.41.0


2023-11-16 22:43:50

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 1/2] kmemleak: drop (age <increasing>) from leak record

Displaying age is pretty, but counter-productive; it changes with
current-time, so it surrenders idempotency of the output, which breaks
simple hash-based cataloging of the records by the user.

The trouble: sequential reads, wo new leaks, get new results:

:#> sum /sys/kernel/debug/kmemleak
53439 74 /sys/kernel/debug/kmemleak
:#> sum /sys/kernel/debug/kmemleak
59066 74 /sys/kernel/debug/kmemleak

and age is why (nothing else changes):

:#> grep -v age /sys/kernel/debug/kmemleak | sum
58894 67
:#> grep -v age /sys/kernel/debug/kmemleak | sum
58894 67

Since jiffies is already printed in the "comm" line, age adds nothing.

Notably, syzkaller reads kmemleak only for "unreferenced object", and
won't care about this reform of age-ism. A few moribund github repos
mention it, but don't compile.

Signed-off-by: Jim Cromie <[email protected]>
---
mm/kmemleak.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 1eacca03bedd..10c9b611c395 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -355,14 +355,12 @@ static void print_unreferenced(struct seq_file *seq,
int i;
unsigned long *entries;
unsigned int nr_entries;
- unsigned int msecs_age = jiffies_to_msecs(jiffies - object->jiffies);

nr_entries = stack_depot_fetch(object->trace_handle, &entries);
warn_or_seq_printf(seq, "unreferenced object 0x%08lx (size %zu):\n",
object->pointer, object->size);
- warn_or_seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu (age %d.%03ds)\n",
- object->comm, object->pid, object->jiffies,
- msecs_age / 1000, msecs_age % 1000);
+ warn_or_seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
+ object->comm, object->pid, object->jiffies);
hex_dump_object(seq, object);
warn_or_seq_printf(seq, " backtrace:\n");

--
2.41.0

2023-11-16 22:43:56

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 2/2] kmemleak: add checksum to backtrace report

Change /sys/kernel/debug/kmemleak report format slightly, adding
"(extra info)" to the backtrace header:

from: " backtrace:"
to: " backtrace (crc <cksum>):"

The <cksum> allows a user to see recurring backtraces without
detailed/careful reading of multiline stacks. So after cycling
kmemleak-test a few times, I know some leaks are repeating.

bash-5.2# grep backtrace /sys/kernel/debug/kmemleak | wc
62 186 1792
bash-5.2# grep backtrace /sys/kernel/debug/kmemleak | sort -u | wc
37 111 1067

syzkaller parses kmemleak for "unreferenced object" only, so is
unaffected by this change. Other github repos are moribund.

Signed-off-by: Jim Cromie <[email protected]>
---
mm/kmemleak.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 10c9b611c395..4c22a2d7cab4 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -362,7 +362,7 @@ static void print_unreferenced(struct seq_file *seq,
warn_or_seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
object->comm, object->pid, object->jiffies);
hex_dump_object(seq, object);
- warn_or_seq_printf(seq, " backtrace:\n");
+ warn_or_seq_printf(seq, " backtrace (crc %x):\n", object->checksum);

for (i = 0; i < nr_entries; i++) {
void *ptr = (void *)entries[i];
--
2.41.0

2023-11-16 23:23:18

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH 0/2] tweak kmemleak report format

On Thu, Nov 16, 2023 at 3:43 PM Jim Cromie <[email protected]> wrote:
>
> These 2 patches make minor changes to the report:
>
> 1st strips "age <increasing>" from output. This makes the output
> idempotent; unchanging until a new leak is reported.
>
> 2nd adds the backtrace.checksum to the "backtrace:" line. This lets a
> user see repeats without actually reading the whole backtrace. So now
> the backtrace line looks like this:
>
> backtrace (ck 603070071):

meh, in the actual patch, thats

backtrace (crc <%x format>)

2023-11-18 17:36:50

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH 1/2] kmemleak: drop (age <increasing>) from leak record

On Thu, Nov 16, 2023 at 03:43:17PM -0700, Jim Cromie wrote:
> Displaying age is pretty, but counter-productive; it changes with
> current-time, so it surrenders idempotency of the output, which breaks
> simple hash-based cataloging of the records by the user.
>
> The trouble: sequential reads, wo new leaks, get new results:
>
> :#> sum /sys/kernel/debug/kmemleak
> 53439 74 /sys/kernel/debug/kmemleak
> :#> sum /sys/kernel/debug/kmemleak
> 59066 74 /sys/kernel/debug/kmemleak
>
> and age is why (nothing else changes):
>
> :#> grep -v age /sys/kernel/debug/kmemleak | sum
> 58894 67
> :#> grep -v age /sys/kernel/debug/kmemleak | sum
> 58894 67
>
> Since jiffies is already printed in the "comm" line, age adds nothing.
>
> Notably, syzkaller reads kmemleak only for "unreferenced object", and
> won't care about this reform of age-ism. A few moribund github repos
> mention it, but don't compile.
>
> Signed-off-by: Jim Cromie <[email protected]>

Reviewed-by: Catalin Marinas <[email protected]>

2023-11-18 17:37:04

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH 2/2] kmemleak: add checksum to backtrace report

On Thu, Nov 16, 2023 at 03:43:18PM -0700, Jim Cromie wrote:
> Change /sys/kernel/debug/kmemleak report format slightly, adding
> "(extra info)" to the backtrace header:
>
> from: " backtrace:"
> to: " backtrace (crc <cksum>):"
>
> The <cksum> allows a user to see recurring backtraces without
> detailed/careful reading of multiline stacks. So after cycling
> kmemleak-test a few times, I know some leaks are repeating.
>
> bash-5.2# grep backtrace /sys/kernel/debug/kmemleak | wc
> 62 186 1792
> bash-5.2# grep backtrace /sys/kernel/debug/kmemleak | sort -u | wc
> 37 111 1067
>
> syzkaller parses kmemleak for "unreferenced object" only, so is
> unaffected by this change. Other github repos are moribund.
>
> Signed-off-by: Jim Cromie <[email protected]>

Reviewed-by: Catalin Marinas <[email protected]>

2023-11-26 06:45:46

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH 2/2] kmemleak: add checksum to backtrace report

On Sat, Nov 18, 2023 at 10:36 AM Catalin Marinas
<[email protected]> wrote:
>
> On Thu, Nov 16, 2023 at 03:43:18PM -0700, Jim Cromie wrote:
> > Change /sys/kernel/debug/kmemleak report format slightly, adding
> > "(extra info)" to the backtrace header:
> >
> > from: " backtrace:"
> > to: " backtrace (crc <cksum>):"
> >
> > The <cksum> allows a user to see recurring backtraces without
> > detailed/careful reading of multiline stacks. So after cycling
> > kmemleak-test a few times, I know some leaks are repeating.
> >
> > bash-5.2# grep backtrace /sys/kernel/debug/kmemleak | wc
> > 62 186 1792
> > bash-5.2# grep backtrace /sys/kernel/debug/kmemleak | sort -u | wc
> > 37 111 1067

So, speculating from here,
what else could be done with <crc: deadbeef> ?

1 - (optionally) collapsing backtraces, replacing the stack with
"seen previously, at <mumble>"
of some clear / succinct flavor (maybe several ?)

2 - stack specific instructions from user

echo drop/ignore/histogram/<mumble> deadbeef \
> /sys/kernel/debug/kmemleak

this crc-specific instruction could control the optionality of 1.
on a trace-by-trace basis even.

The "seen previously" would be an obvious place to look
for a root cause of a detected leak.
tools beyond drop/ignore/histogram/<mumble>
are worth some consideration ?

> Reviewed-by: Catalin Marinas <[email protected]>