Re: [PATCH v6 0/2] Return EADDRNOTAVAIL when func matches several symbols during kprobe creation

[Francis Laniel <flaniel@linux.microsoft.com>]

Hi!

Le vendredi 20 octobre 2023, 15:12:39 EEST Masami Hiramatsu a écrit :
> Hi,
> 
> Thanks for update the series. The series looks good to me.
> Let me pick those in probes/fixes.

Thank you for picking it and all the good advices during the development 
process!

> Thank you!

Best regards.

> On Fri, 20 Oct 2023 13:42:48 +0300
> 
> Francis Laniel <flaniel@linux.microsoft.com> wrote:
> > Hi.
> > 
> > 
> > In the kernel source code, it exists different functions which share the
> > same name but which have, of course, different addresses as they can be
> > defined in different modules:
> > # Kernel was compiled with CONFIG_NTFS_FS and CONFIG_NTFS3_FS as built-in.
> > root@vm-amd64:~# grep ntfs_file_write_iter /proc/kallsyms
> > ffffffff814ce3c0 t __pfx_ntfs_file_write_iter
> > ffffffff814ce3d0 t ntfs_file_write_iter
> > ffffffff814fc8a0 t __pfx_ntfs_file_write_iter
> > ffffffff814fc8b0 t ntfs_file_write_iter
> > This can be source of troubles when you create a PMU kprobe for such a
> > function, as it will only install one for the first address (e.g.
> > 0xffffffff814ce3d0 in the above).
> > This could lead to some troubles were BPF based tools does not report any
> > event because the second function is not called:
> > root@vm-amd64:/mnt# mount | grep /mnt
> > /foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8)
> > # ig is a tool which installs a PMU kprobe on ntfs_file_write_iter().
> > root@vm-amd64:/mnt# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo &
> > [1] 207
> > root@vm-amd64:/mnt# dd if=./foo of=./bar count=3
> > 3+0 records in
> > 3+0 records out
> > 1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00543323 s, 283 kB/s
> > root@vm-amd64:/mnt# fg
> > ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo
> > ^Croot@vm-amd64:/mnt# more /tmp/foo
> > RUNTIME.CONTAINERNAME          RUNTIME.CONTAIN… PID              COMM
> > 
> >   T      BYTES     OFFSET        LAT FILE
> >   
> >                                                 214              dd
> >   
> >   R        512          0        766 foo
> >   
> >                                                 214              dd
> >   
> >   R        512        512          9 foo
> >   
> >                                                 214              dd
> > 
> > As you can see in the above, only read events are reported and no write
> > because the kprobe is installed for the old ntfs_file_write_iter() and
> > not the ntfs3 one.
> > The same behavior occurs with sysfs kprobe:
> > root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter'
> > > /sys/kernel/tracing/kprobe_events root@vm-amd64:/# cat
> > /sys/kernel/tracing/kprobe_events
> > p:probe/ntfs_file_write_iter ntfs_file_write_iter
> > root@vm-amd64:/# mount | grep /mnt
> > /foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8)
> > root@vm-amd64:/# perf record -e probe:ntfs_file_write_iter &
> > [1] 210
> > root@vm-amd64:/# cd /mnt/
> > root@vm-amd64:/mnt# dd if=./foo of=./bar count=3
> > 3+0 records in
> > 3+0 records out
> > 1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00234793 s, 654 kB/s
> > root@vm-amd64:/mnt# cd -
> > /
> > root@vm-amd64:/# fg
> > perf record -e probe:ntfs_file_write_iter
> > ^C[ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.056 MB perf.data ]
> > 
> > root@vm-amd64:/# perf report
> > Error:
> > The perf.data data has no samples!
> > # To display the perf.data header info, please use --header/--header-only
> > optio> #
> > 
> > In this contribution, I modified the functions creating sysfs and PMU
> > kprobes to test if the function name given as argument matches several
> > symbols. In this case, these functions return EADDRNOTAVAIL to indicate
> > the user to use addr and offs to remove this ambiguity.
> > So, when the above BPF tool is run, the following error message is
> > printed:
> > root@vm-amd64:~# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo &
> > [1] 228
> > root@vm-amd64:~# more /tmp/foo
> > RUNTIME.CONTAINERNAME          RUNTIME.CONTAIN… PID              COMM
> > 
> >   T      BYTES     OFFSET        LAT FILE
> > 
> > Error: running gadget: running gadget: installing tracer: attaching
> > kprobe: crea ting perf_kprobe PMU (arch-specific fallback for
> > "ntfs_file_write_iter"): token ntfs_file_write_iter: opening perf event:
> > cannot assign requested address And the same with sysfs kprobe:
> > root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter'
> > > /sys/kernel/tracing/kprobe_events -bash: echo: write error: Cannot
> > assign requested address
> > Note that, this does not influence perf as it installs kprobes as offset
> > on
> > _text:
> > root@vm-amd64:/# perf probe --add ntfs_file_write_iter
> > 
> > Added new events:
> >   probe:ntfs_file_write_iter (on ntfs_file_write_iter)
> >   probe:ntfs_file_write_iter (on ntfs_file_write_iter)
> > 
> > ...
> > root@vm-amd64:/# cat /sys/kernel/tracing/kprobe_events
> > p:probe/ntfs_file_write_iter _text+5039088
> > p:probe/ntfs_file_write_iter _text+5228752
> > 
> > Note that, this contribution is the conclusion of a previous RFC which
> > intended to install a PMU kprobe for all matching symbols [1, 2].
> > 
> > If you see any way to improve this contribution, please share your
> > feedback.> 
> > Changes since:
> >  v1:
> >   * Use EADDRNOTAVAIL instead of adding a new error code.
> >   * Correct also this behavior for sysfs kprobe.
> >  
> >  v2:
> >   * Count the number of symbols corresponding to function name and return
> >   EADDRNOTAVAIL if higher than 1.
> >   * Return ENOENT if above count is 0, as it would be returned later by
> >   while
> >   registering the kprobe.
> >  
> >  v3:
> >   * Check symbol does not contain ':' before testing its uniqueness.
> >   * Add a selftest to check this is not possible to install a kprobe for a
> >   non unique symbol.
> >  
> >  v5:
> >   * No changes, just add linux-stable as recipient.
> >  
> >  v6:
> >   * Rephrase commit message.
> >   * Add "Cc:" to stable.
> > 
> > Francis Laniel (2):
> >   tracing/kprobes: Return EADDRNOTAVAIL when func matches several
> >   
> >     symbols
> >   
> >   selftests/ftrace: Add new test case which checks non unique symbol
> >  
> >  kernel/trace/trace_kprobe.c                   | 63 +++++++++++++++++++
> >  kernel/trace/trace_probe.h                    |  1 +
> >  .../test.d/kprobe/kprobe_non_uniq_symbol.tc   | 13 ++++
> >  3 files changed, 77 insertions(+)
> >  create mode 100644
> >  tools/testing/selftests/ftrace/test.d/kprobe/kprobe_non_uniq_symbol.tc> 
> > Best regards and thank you in advance.
> > ---
> > [1]:
> > https://lore.kernel.org/lkml/20230816163517.112518-1-flaniel@linux.micros
> > oft.com/ [2]:
> > https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kern
> > el.org/ --
> > 2.34.1