Re: [RFC] [PATCH bpf-next 1/1] bpf: Clear the noisy tail buffer for bpf_d_path() helper

[xufeng zhang]

Hi Tao,

在 2021/11/24 下午4:49, Hou Tao 写道:
> Hi,
>
> On 11/24/2021 12:15 PM, xufeng zhang wrote:
> > Jiri and KP,
> >
> > Any suggestion?
> >
> >
> > Thanks in advance!
> >
> > Xufeng
> >
> > 在 2021/11/20 下午1:18, Xufeng Zhang 写道:
> > > From: "Xufeng Zhang" <yunbo.xufeng@xxxxxxxxxxxxxxxxx>
> > >
> > > The motivation behind this change is to use the returned full path
> > > for lookup keys in BPF_MAP_TYPE_HASH map.
> > > bpf_d_path() prepend the path string from the end of the input
> > > buffer, and call memmove() to copy the full path from the tail
> > > buffer to the head of buffer before return. So although the
> > > returned buffer string is NULL terminated, there is still
> > > noise data at the tail of buffer.
> > > If using the returned full path buffer as the key of hash map,
> > > the noise data is also calculated and makes map lookup failed.
> > > To resolve this problem, we could memset the noisy tail buffer
> > > before return.
> > >
> > > Signed-off-by: Xufeng Zhang <yunbo.xufeng@xxxxxxxxxxxxxxxxx>
> > > ---
> > >    kernel/trace/bpf_trace.c | 2 ++
> > >    1 file changed, 2 insertions(+)
> > >
> > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > index 25ea521fb8f1..ec4a6823c024 100644
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -903,6 +903,8 @@ BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf,
> > > u32, sz)
> > >        } else {
> > >            len = buf + sz - p;
> > >            memmove(buf, p, len);
> > > +        /* Clear the noisy tail buffer before return */
> > > +        memset(buf + len, 0, sz - len);
> Is implementing bpf_memset() helper a better idea ? So those who need to
> clear the buffer after the terminated null character can use the helper to
> do that.

This is a good point.

I think the reason why mainline has not such a helper yet is because a 
LLVM __builtin_memset() is

already available, but clearly this __builtin_memset() has too much 
limitation which can't meet all the needs,

there might be other concerns to implement such a memset helper which I 
don't know, but I think your suggestion

is a good idea.


Xufeng


> Regards,
> Tao
>
> > >        }
> > >          return len;
> > .