Re: [PATCH 2/3] bpf: Add d_path helper

Florent Revest
2020-04-02T14:03:01Z

On Wed, 2020-04-01 at 13:09 +0200, Jiri Olsa wrote:
> + * int bpf_d_path(struct path *path, char *buf, u32 sz)
> + * Description
> + * Return full path for given 'struct path' object, which
> + * needs to be the kernel BTF 'path' object. The path is
> + * returned in buffer provided 'buf' of size 'sz'.
> + *
> + * Return
> + * length of returned string on success, or a negative
> + * error in case of failure
> + *

You might want to add that d_path is ambiguous since it can add
" (deleted)" at the end of your path and you don't know whether this is
actually part of the file path or not. :)

> +BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
> +{
> + char *p = d_path(path, buf, sz - 1);

I am curious why you'd use sz - 1 here? In my experience, d_path's
output is 0 limited so you shouldn't need to keep an extra byte for
that (if that was the intention here).

> + int len;
> +
> + if (IS_ERR(p)) {
> + len = PTR_ERR(p);
> + } else {
> + len = strlen(p);
> + if (len && p != buf) {
> + memmove(buf, p, len);

Have you considered returning the offset within buf instead and let the
BPF program do pointer arithmetics to find the beginning of the string?

> + buf[len] = 0;

If my previous comment about sz - 1 is true, then this wouldn't be
necessary, you could just use memmove with len + 1.

> + }
> + }
> +
> + return len;
> +}