If format changes are not /sys/** ABI violating, heres 3 minor ones:

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):  # also see below

Q: should ck be spelled crc ? it feels more communicative.

NB: with ck exposed, it becomes possible to do a "selective clear",
something like:

  echo drop 603070071 > /sys/kernel/debug/kmemleak

The 3rd patch takes __init off of kmemleak_test_init().  This fixes a
bare-pointer in the 2nd line of the backtrace below, which previously
looked like:

    [<00000000ef738764>] 0xffffffffc02350a2

NB: this happens still/again, after rmmod kmemleak-test.

unreferenced object 0xffff888005d9ca40 (size 32):
  comm "modprobe", pid 412, jiffies 4294703300
  hex dump (first 32 bytes):
    00 cd d9 05 80 88 ff ff 40 cf d9 05 80 88 ff ff  ........@.......
    14 a7 c4 f6 7d f9 87 10 00 00 00 00 00 00 00 00  ....}...........
  backtrace (ck 1354775490):
    [<000000002c474f61>] kmalloc_trace+0x26/0x90
    [<00000000b26599c1>] kmemleak_test_init+0x58/0x2d0 [kmemleak_test]
    [<0000000044d13990>] do_one_initcall+0x43/0x210
    [<00000000131bc505>] do_init_module+0x4a/0x210
    [<00000000b2902890>] __do_sys_finit_module+0x93/0xf0
    [<00000000673fdce2>] do_syscall_64+0x34/0x80
    [<00000000357a2d80>] entry_SYSCALL_64_after_hwframe+0x46/0xb0


Jim Cromie (3):
  kmemleak: drop (age <increasing>) from leak record
  kmemleak: add checksum to backtrace report
  kmemleak-test: drop __init to get better backtrace

 mm/kmemleak.c                    | 8 +++-----
 samples/kmemleak/kmemleak-test.c | 2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.40.0

On Tue, Apr 25, 2023 at 04:24:43PM -0600, Jim Cromie wrote:
> If format changes are not /sys/** ABI violating, heres 3 minor ones:
> 
> 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):  # also see below
> 
> Q: should ck be spelled crc ? it feels more communicative.

These all would make sense (and 'crc' sounds better) if they were done
from the start. I know there are test scripts out there parsing the
kmemleak sysfs file. I can't tell whether these changes would break
them.

Cc'ing Dmitry, I think syzbot was regularly checking kmemleak (not sure
it still does).

> NB: with ck exposed, it becomes possible to do a "selective clear",
> something like:
> 
>   echo drop 603070071 > /sys/kernel/debug/kmemleak
> 
> The 3rd patch takes __init off of kmemleak_test_init().  This fixes a
> bare-pointer in the 2nd line of the backtrace below, which previously
> looked like:
> 
>     [<00000000ef738764>] 0xffffffffc02350a2
> 
> NB: this happens still/again, after rmmod kmemleak-test.
> 
> unreferenced object 0xffff888005d9ca40 (size 32):
>   comm "modprobe", pid 412, jiffies 4294703300
>   hex dump (first 32 bytes):
>     00 cd d9 05 80 88 ff ff 40 cf d9 05 80 88 ff ff  ........@.......
>     14 a7 c4 f6 7d f9 87 10 00 00 00 00 00 00 00 00  ....}...........
>   backtrace (ck 1354775490):
>     [<000000002c474f61>] kmalloc_trace+0x26/0x90
>     [<00000000b26599c1>] kmemleak_test_init+0x58/0x2d0 [kmemleak_test]
>     [<0000000044d13990>] do_one_initcall+0x43/0x210
>     [<00000000131bc505>] do_init_module+0x4a/0x210
>     [<00000000b2902890>] __do_sys_finit_module+0x93/0xf0
>     [<00000000673fdce2>] do_syscall_64+0x34/0x80
>     [<00000000357a2d80>] entry_SYSCALL_64_after_hwframe+0x46/0xb0

-- 
Catalin

On Fri, Apr 28, 2023 at 11:25 AM Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> On Tue, Apr 25, 2023 at 04:24:43PM -0600, Jim Cromie wrote:
> > If format changes are not /sys/** ABI violating, heres 3 minor ones:
> >
> > 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):  # also see below
> >
> > Q: should ck be spelled crc ? it feels more communicative.
>
> These all would make sense (and 'crc' sounds better) if they were done
> from the start. I know there are test scripts out there parsing the
> kmemleak sysfs file. I can't tell whether these changes would break
> them.
>
> Cc'ing Dmitry, I think syzbot was regularly checking kmemleak (not sure
> it still does).
>

I took a look at syzkaller repo, found kmemleak parsing in
executor/common_linux.h
in static void check_leaks(char** frames, int nframes)

this parse just counts occurrences of "unreferenced object",
it does not expect to find "age" nor would it (apparently) choke on
"crc" being added.


There are also a few kmemleak refs in several github repos
3 have updates since 2020.

perf-monitor  has 2 forks, both have the same minor compile
warning-turned-error.
umemleak makes mention of kmemleak, but it has no code, just a readme.

QED: there are no kmemleak parsers in public github repos that would
break with these changes

While there may be parsers on the dark-web,
ISTM none could be relying upon "age" in any meaningful way.
and none are likely to choke because "(crc: <foo>)"
was added to the stack trace display.

If these patches spend some time in purgatory (in linux-next or
linux-eventually)
perhaps the remaining risks can be discounted ?








> > NB: with ck exposed, it becomes possible to do a "selective clear",
> > something like:
> >
> >   echo drop 603070071 > /sys/kernel/debug/kmemleak
> >
> > The 3rd patch takes __init off of kmemleak_test_init().  This fixes a
> > bare-pointer in the 2nd line of the backtrace below, which previously
> > looked like:
> >
> >     [<00000000ef738764>] 0xffffffffc02350a2
> >
> > NB: this happens still/again, after rmmod kmemleak-test.
> >
> > unreferenced object 0xffff888005d9ca40 (size 32):
> >   comm "modprobe", pid 412, jiffies 4294703300
> >   hex dump (first 32 bytes):
> >     00 cd d9 05 80 88 ff ff 40 cf d9 05 80 88 ff ff  ........@.......
> >     14 a7 c4 f6 7d f9 87 10 00 00 00 00 00 00 00 00  ....}...........
> >   backtrace (ck 1354775490):
> >     [<000000002c474f61>] kmalloc_trace+0x26/0x90
> >     [<00000000b26599c1>] kmemleak_test_init+0x58/0x2d0 [kmemleak_test]
> >     [<0000000044d13990>] do_one_initcall+0x43/0x210
> >     [<00000000131bc505>] do_init_module+0x4a/0x210
> >     [<00000000b2902890>] __do_sys_finit_module+0x93/0xf0
> >     [<00000000673fdce2>] do_syscall_64+0x34/0x80
> >     [<00000000357a2d80>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
>
> --
> Catalin

On Fri, Nov 10, 2023 at 05:19:38PM -0700, jim.cromie@gmail.com wrote:
> On Fri, Apr 28, 2023 at 11:25 AM Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > On Tue, Apr 25, 2023 at 04:24:43PM -0600, Jim Cromie wrote:
> > > If format changes are not /sys/** ABI violating, heres 3 minor ones:
> > >
> > > 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):  # also see below
> > >
> > > Q: should ck be spelled crc ? it feels more communicative.
> >
> > These all would make sense (and 'crc' sounds better) if they were done
> > from the start. I know there are test scripts out there parsing the
> > kmemleak sysfs file. I can't tell whether these changes would break
> > them.
> >
> > Cc'ing Dmitry, I think syzbot was regularly checking kmemleak (not sure
> > it still does).
[...]
> QED: there are no kmemleak parsers in public github repos that would
> break with these changes

Thanks for digging into this, I completely forgot about this series.
Would you mind rebasing to the latest kernel and reposting?

Thanks.

-- 
Catalin