Re: [PATCH bpf-next 1/2] bpf: Add a bpf_kallsyms_lookup helper

[Yonghong Song]

On 12/11/20 6:40 AM, Florent Revest wrote:
> On Wed, Dec 2, 2020 at 10:18 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> > I still think that adopting printk/vsnprintf for this instead of
> > reinventing the wheel
> > is more flexible and easier to maintain long term.
> > Almost the same layout can be done with vsnprintf
> > with exception of \0 char.
> > More meaningful names, etc.
> > See Documentation/core-api/printk-formats.rst
>
> I agree this would be nice. I finally got a bit of time to experiment
> with this and I noticed a few things:
>
> First of all, because helpers only have 5 arguments, if we use two for
> the output buffer and its size and two for the format string and its
> size, we are only left with one argument for a modifier. This is still
> enough for our usecase (where we'd only use "%ps" for example) but it
> does not strictly-speaking allow for the same layout that Andrii
> proposed.

See helper bpf_seq_printf. It packs all arguments for format string and
puts them into an array. bpf_seq_printf will unpack them as it parsed
through the format string. So it should be doable to have more than
"%ps" in format string.

> > If we force fmt to come from readonly map then bpf_trace_printk()-like
> > run-time check of fmt string can be moved into load time check
> > and performance won't suffer.
>
> Regarding this bit, I have the impression that this would not be
> possible, but maybe I'm missing something ? :)
>
> The iteration that bpf_trace_printk does over the format string
> argument is not only used for validation. It is also used to remember
> what extra operations need to be done based on the modifier types. For
> example, it remembers whether an arg should be interpreted as 32bits or
> 64bits. In the case of string printing, it also remembers whether it is
> a kernel-space or user-space pointer so that bpf_trace_copy_string can
> be called with the right arg. If we were to run the iteration over the format
> string in the verifier, how would you recommend that we
> "remember" the modifier type until the helper gets called ?