Re: [PATCH 0/3] kmemleak report format changes

[jim . cromie]

On Fri, Apr 28, 2023 at 11:25 AM Catalin Marinas
<catalin.marinas@xxxxxxx> 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