Re: [PATCH v8 bpf-next 09/13] bpf: Add d_path helper

[Al Viro <viro@zeniv.linux.org.uk>]

On Wed, Jul 22, 2020 at 11:12:19PM +0200, Jiri Olsa wrote:

> +BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
> +{
> +	char *p = d_path(path, buf, sz - 1);
> +	int len;
> +
> +	if (IS_ERR(p)) {
> +		len = PTR_ERR(p);
> +	} else {
> +		len = strlen(p);
> +		if (len && p != buf)
> +			memmove(buf, p, len);

*blink*
What the hell do you need that strlen() for?  d_path() copies into
the end of buffer (well, starts there and prepends to it); all you
really need is memmove(buf, p, buf + sz - p)


> +		buf[len] = 0;

Wait a minute...  Why are you NUL-terminating it separately?
You do rely upon having NUL in the damn thing (and d_path() does
guarantee it there).  Without that strlen() would've gone into
the nasal demon country; you can't call it on non-NUL-terminated
array.  So you are guaranteed that p[len] will be '\0'; why bother
copying the first len bytes and then separately deal with that
NUL?  Just memmove() the fucker and be done with that...

If you are worried about stray NUL in the middle of the returned
data... can't happen.  Note the rename_lock use in fs/d_path.c;
the names of everything involved are guaranteed to have been
stable throughout the copying them into the buffer - if anything
were to be renamed while we are doing that, we'd repeat the whole
thing (with rename_lock taken exclusive the second time around).

So make it simply
	if (IS_ERR(p))
		return PTR_ERR(p);
	len = buf + sz - p;
	memmove(buf, p, len);
	return len;
and be done with that.  BTW, the odds of p == buf are pretty much
nil - it would happen only if sz - 1 happened to be the exact length
of pathname.