[PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Suchit Karunakaran]

Replace the unsafe strcpy() call with memcpy() when copying the path
into the bpf_object structure. Since the memory is pre-allocated to
exactly strlen(path) + 1 bytes and the length is already known, memcpy()
is safer than strcpy().

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
 tools/lib/bpf/libbpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 52e353368f58..279f226dd965 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1495,7 +1495,7 @@ static struct bpf_object *bpf_object__new(const char *path,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	strcpy(obj->path, path);
+	memcpy(obj->path, path, strlen(path) + 1);
 	if (obj_name) {
 		libbpf_strlcpy(obj->name, obj_name, sizeof(obj->name));
 	} else {
-- 
2.50.1

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Yonghong Song]

On 7/17/25 4:59 AM, Suchit Karunakaran wrote:
> Replace the unsafe strcpy() call with memcpy() when copying the path
> into the bpf_object structure. Since the memory is pre-allocated to
> exactly strlen(path) + 1 bytes and the length is already known, memcpy()
> is safer than strcpy().

I don't understand in this particular context why strcpy()
is less safer than memcpy(). Both of them will achieve the
exactly same goal.

>
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>   tools/lib/bpf/libbpf.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 52e353368f58..279f226dd965 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1495,7 +1495,7 @@ static struct bpf_object *bpf_object__new(const char *path,
>   		return ERR_PTR(-ENOMEM);
>   	}
>   
> -	strcpy(obj->path, path);
> +	memcpy(obj->path, path, strlen(path) + 1);
>   	if (obj_name) {
>   		libbpf_strlcpy(obj->name, obj_name, sizeof(obj->name));
>   	} else {

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Suchit K]

On Thu, 17 Jul 2025 at 22:19, Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
> On 7/17/25 4:59 AM, Suchit Karunakaran wrote:
> > Replace the unsafe strcpy() call with memcpy() when copying the path
> > into the bpf_object structure. Since the memory is pre-allocated to
> > exactly strlen(path) + 1 bytes and the length is already known, memcpy()
> > is safer than strcpy().
>
> I don't understand in this particular context why strcpy()
> is less safer than memcpy(). Both of them will achieve the
> exactly same goal.
>

Sorry, I meant that strcpy() is generally considered unsafe because it
doesn't perform bounds checking. Its use is deprecated and
discouraged, as noted in Documentation/process/deprecated.rst. I made
this change with that in mind, although I'm not entirely certain
whether it's actually unsafe in this specific context.

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Greg KH]

On Thu, Jul 17, 2025 at 10:29:50PM +0530, Suchit K wrote:
> On Thu, 17 Jul 2025 at 22:19, Yonghong Song <yonghong.song@linux.dev> wrote:
> >
> >
> >
> > On 7/17/25 4:59 AM, Suchit Karunakaran wrote:
> > > Replace the unsafe strcpy() call with memcpy() when copying the path
> > > into the bpf_object structure. Since the memory is pre-allocated to
> > > exactly strlen(path) + 1 bytes and the length is already known, memcpy()
> > > is safer than strcpy().
> >
> > I don't understand in this particular context why strcpy()
> > is less safer than memcpy(). Both of them will achieve the
> > exactly same goal.
> >
> 
> Sorry, I meant that strcpy() is generally considered unsafe because it
> doesn't perform bounds checking. Its use is deprecated and
> discouraged, as noted in Documentation/process/deprecated.rst. I made
> this change with that in mind, although I'm not entirely certain
> whether it's actually unsafe in this specific context.
> 

Your change also did not do any bounds checking at all, so how is this
now safer?

confused,

greg k-h

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Suchit K]

> Your change also did not do any bounds checking at all, so how is this
> now safer?
>
> confused,
>
> greg k-h

I assumed bounds checking wasn't necessary here because obj is
allocated at the start of the function with enough space
(sizeof(struct bpf_object) + strlen(path) + 1). My main motivation for
the change was the deprecation of strcpy(). However, thinking about it
now, I'm not entirely sure memcpy is even needed in this context. I'd
really appreciate any feedback or clarification on the best approach
here.

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Andrii Nakryiko]

On Thu, Jul 17, 2025 at 4:59 AM Suchit Karunakaran
<suchitkarunakaran@gmail.com> wrote:
>
> Replace the unsafe strcpy() call with memcpy() when copying the path
> into the bpf_object structure. Since the memory is pre-allocated to
> exactly strlen(path) + 1 bytes and the length is already known, memcpy()
> is safer than strcpy().
>
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>  tools/lib/bpf/libbpf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 52e353368f58..279f226dd965 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1495,7 +1495,7 @@ static struct bpf_object *bpf_object__new(const char *path,
>                 return ERR_PTR(-ENOMEM);
>         }
>
> -       strcpy(obj->path, path);
> +       memcpy(obj->path, path, strlen(path) + 1);


This is user-space libbpf code, where the API contract mandates that
the path argument is a well-formed zero-terminated C string. Plus, if
you look at the few lines above, we allocate just enough space to fit
the entire contents of the string without truncation.

In other words, there is nothing to fix or improve here.

pw-bot: cr

>         if (obj_name) {
>                 libbpf_strlcpy(obj->name, obj_name, sizeof(obj->name));
>         } else {
> --
> 2.50.1
>

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Suchit K]

On Thu, 17 Jul 2025 at 22:49, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jul 17, 2025 at 4:59 AM Suchit Karunakaran
> <suchitkarunakaran@gmail.com> wrote:
> >
> > Replace the unsafe strcpy() call with memcpy() when copying the path
> > into the bpf_object structure. Since the memory is pre-allocated to
> > exactly strlen(path) + 1 bytes and the length is already known, memcpy()
> > is safer than strcpy().
> >
> > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > ---
> >  tools/lib/bpf/libbpf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 52e353368f58..279f226dd965 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -1495,7 +1495,7 @@ static struct bpf_object *bpf_object__new(const char *path,
> >                 return ERR_PTR(-ENOMEM);
> >         }
> >
> > -       strcpy(obj->path, path);
> > +       memcpy(obj->path, path, strlen(path) + 1);
>
>
> This is user-space libbpf code, where the API contract mandates that
> the path argument is a well-formed zero-terminated C string. Plus, if
> you look at the few lines above, we allocate just enough space to fit
> the entire contents of the string without truncation.
>
> In other words, there is nothing to fix or improve here.
>
> pw-bot: cr
>

That makes sense, strcpy() is indeed safe here. Thanks for the clarification.

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Suchit K]

> This is user-space libbpf code, where the API contract mandates that
> the path argument is a well-formed zero-terminated C string. Plus, if
> you look at the few lines above, we allocate just enough space to fit
> the entire contents of the string without truncation.
>
> In other words, there is nothing to fix or improve here.
>

Even though it’s safe in this context, would it still be a good idea
to replace strcpy() with something like memcpy() since it's
deprecated? I’m still a beginner in kernel development and trying to
find my way around, so I’d appreciate any guidance.

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Andrii Nakryiko]

On Thu, Jul 17, 2025 at 10:33 AM Suchit K <suchitkarunakaran@gmail.com> wrote:
>
> > This is user-space libbpf code, where the API contract mandates that
> > the path argument is a well-formed zero-terminated C string. Plus, if
> > you look at the few lines above, we allocate just enough space to fit
> > the entire contents of the string without truncation.
> >
> > In other words, there is nothing to fix or improve here.
> >
>
> Even though it’s safe in this context, would it still be a good idea
> to replace strcpy() with something like memcpy() since it's

no, there is no need. And keep in mind that this is libbpf library
source code, which is developed as part of kernel repo, but isn't
running inside the kernel itself

> deprecated? I’m still a beginner in kernel development and trying to
> find my way around, so I’d appreciate any guidance.

Re: [PATCH] libbpf: Replace strcpy() with memcpy() in bpf_object__new()

[Suchit K]

On Fri, 18 Jul 2025 at 21:09, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jul 17, 2025 at 10:33 AM Suchit K <suchitkarunakaran@gmail.com> wrote:
> >
> > > This is user-space libbpf code, where the API contract mandates that
> > > the path argument is a well-formed zero-terminated C string. Plus, if
> > > you look at the few lines above, we allocate just enough space to fit
> > > the entire contents of the string without truncation.
> > >
> > > In other words, there is nothing to fix or improve here.
> > >
> >
> > Even though it’s safe in this context, would it still be a good idea
> > to replace strcpy() with something like memcpy() since it's
>
> no, there is no need. And keep in mind that this is libbpf library
> source code, which is developed as part of kernel repo, but isn't
> running inside the kernel itself
>

Yup got it. Thank you so much for the clarification.