Re: [PATCH v4] libbpf: Replace strncpy() with strnlen()+memcpy() in skel_map_create()

[Kees Cook]

On Thu, Apr 02, 2026 at 11:14:58AM -0700, Alexei Starovoitov wrote:
> On Tue, Mar 31, 2026 at 5:11 PM Kees Cook <kees@xxxxxxxxxx> wrote:
> >
> > BPF is the last user of the deprecated[1] strncpy() in the kernel.
> > Replace it with a length check via strnlen() on the source followed
> > by memcpy(). Normally strscpy() would be used in this case, but
> > skel_internal.h is shared between kernel and userspace tools, and
> > strscpy() is not available in the userspace build context.
> >
> > The source map_name is a NUL-terminated C string (the only caller
> > passes the "__loader.map" 12 character string literal). The destination
> > attr.map_name is char[BPF_OBJ_NAME_LEN] (16 bytes) in union bpf_attr,
> > ultimately passed to the bpf() syscall.
> >
> > The bpf(BPF_MAP_CREATE) syscall, through bpf_obj_name_cpy(), requires a
> > NUL terminator within this 16-byte array, rejecting names that use all 16
> > bytes. Valid names are therefore at most 15 characters, but this wasn't
> > being checked via the skel_map_create() path. Add a matching check and
> > refuse 16+ character strings early, as they would be refused later by
> > bpf_obj_name_cpy().
> >
> > The attr is pre-zeroed with memset() at the top of the function, so
> > the last byte of attr.map_name is always NUL, meaning the memcpy()
> > of just the non-NUL characters from the source will always produce a
> > NUL-terminated destination string.
> >
> > Link: https://github.com/KSPP/linux/issues/90 [1]
> > Suggested-by: Sun Jian <sun.jian.kdev@xxxxxxxxx>
> > Signed-off-by: Kees Cook <kees@xxxxxxxxxx>
> > ---
> >  v4: Reformat commit log slightly, confirm last user of strnlen in -next
> >  v3: https://lore.kernel.org/lkml/20260324161605.make.168-kees@xxxxxxxxxx/
> >  v2: https://lore.kernel.org/lkml/20260324053036.it.906-kees@xxxxxxxxxx/
> >  v1: https://lore.kernel.org/lkml/20260324040535.work.851-kees@xxxxxxxxxx/
> > Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
> > Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> > Cc: sun jian <sun.jian.kdev@xxxxxxxxx>
> > Cc: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > Cc: Eduard Zingerman <eddyz87@xxxxxxxxx>
> > Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> > Cc: Martin KaFai Lau <martin.lau@xxxxxxxxx>
> > Cc: Song Liu <song@xxxxxxxxxx>
> > Cc: Yonghong Song <yonghong.song@xxxxxxxxx>
> > Cc: John Fastabend <john.fastabend@xxxxxxxxx>
> > Cc: KP Singh <kpsingh@xxxxxxxxxx>
> > Cc: Stanislav Fomichev <sdf@xxxxxxxxxxx>
> > Cc: Hao Luo <haoluo@xxxxxxxxxx>
> > Cc: <bpf@xxxxxxxxxxxxxxx>
> > ---
> >  tools/lib/bpf/skel_internal.h | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h
> > index 6a8f5c7a02eb..2d38c387f43c 100644
> > --- a/tools/lib/bpf/skel_internal.h
> > +++ b/tools/lib/bpf/skel_internal.h
> > @@ -236,6 +236,7 @@ static inline int skel_map_create(enum bpf_map_type map_type,
> >  {
> >         const size_t attr_sz = offsetofend(union bpf_attr, excl_prog_hash_size);
> >         union bpf_attr attr;
> > +       size_t map_name_len;
> >
> >         memset(&attr, 0, attr_sz);
> >
> > @@ -243,7 +244,12 @@ static inline int skel_map_create(enum bpf_map_type map_type,
> >         attr.excl_prog_hash = (unsigned long) excl_prog_hash;
> >         attr.excl_prog_hash_size = excl_prog_hash_sz;
> >
> > -       strncpy(attr.map_name, map_name, sizeof(attr.map_name));
> > +       /* attr.map_name must be NUL-terminated, like bpf_obj_name_cpy() */
> > +       map_name_len = strnlen(map_name, sizeof(attr.map_name));
> > +       if (map_name_len == sizeof(attr.map_name))
> > +               return -EINVAL;
> > +       memcpy(attr.map_name, map_name, map_name_len);
> 
> this is plain ugly and inefficient.
> Just #ifdef kernel and use strscpy in that branch.

I think you're asking for:

#ifdef __KERNEL__
	if (strscpy(attr.map_name, map_name) < 0)
		return -EINVAL;
#else
	strncpy(attr.map_name, map_name, sizeof(attr.map_name));
#endif

Is that right?

-- 
Kees Cook