Re: [PATCH v4 4/7] uaccess-buffer: add CONFIG_GENERIC_ENTRY support

Peter,

On Thu, Dec 16 2021 at 16:09, Peter Collingbourne wrote:

userspace program sets addr to 0. Furthermore, a userspace program
setting addr to 0 would not automatically cause the pending signals to
be delivered (because simply storing a value to memory from userspace
will not necessarily trigger a kernel entry), and signals could
therefore be left blocked for longer than expected (at least until the
next kernel entry).

Groan, so what you are trying to prevent is:

   *ptr = addr;

--> interrupt
       signal raised

    signal delivery

    signal handler
      syscall()   <- Logs this syscall
      sigreturn;

   syscall() <- Is not logged

I must have missed that detail in these novel sized comments all over
the place.

Yes, I can see how that pre/post muck solves this, but TBH while it is
admittedly a smart hack it's also a horrible hack.

There are a two aspects which I really dislike:

  - It's yet another ad hoc 'solution' to scratch 'my particular itch'

  - It's adding a horrorshow in the syscall hotpath. We have already
    enough gunk there. No need to add more.

The problem you are trying to solve is to instrument user accesses of
the kernel, which is special purpose tracing, right?

May I ask why this is not solvable via tracepoints?

  DECLARE_EVENT_CLASS(uaccess_class,....);
  DECLARE_EVENT(uaccess_class, uaccess_read,...);
  DECLARE_EVENT(uaccess_class, uaccess_write,...);

    trace_uaccess_read(from, n);

    trace_uaccess_write(to, n);

Tracepoints have filters, tooling, libraries etc. Zero code except for
the tracepoints themself. They are disabled by default with a static
key, which means very close to 0 overhead.

Aside of that such tracepoints can be used for other purposes as well
and are therefore not bound to 'my particular itch'.

There are obviously some questions to solve versus filtering, but even
with a stupid tid based filter, it's easy enough to filter out the stuff
you're interested in. E.g. to filter out the signal scenario above all
you need is to enable two more tracepoints:

   signal:signal_deliver
   syscalls:sys_enter_rt_sigreturn

and when analyzing the event stream you can just skip the noise between
signal:signal_deliver and syscalls:sys_enter_rt_sigreturn trace entries.

There are other fancies like BPF which can be used for filtering and
filling a map with entries.

The only downside is that system configuration restricts access and
requires certain priviledges. But is that a real problem for the purpose
at hand, sanitizers and validation tools?

I don't think it is, but you surely can provide more information here.

Thanks,

        tglx

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel