[PATCH v2] libbpf: fix the name of a reused map

[PATCH v2] libbpf: fix the name of a reused map

[Anquan Wu]

BPF map name is limited to BPF_OBJ_NAME_LEN.
A map name is defined as being longer than BPF_OBJ_NAME_LEN,
it will be truncated to BPF_OBJ_NAME_LEN when a userspace program
calls libbpf to create the map. A pinned map also generates a path
in the /sys. If the previous program wanted to reuse the map，
it can not get bpf_map by name, because the name of the map is only
partially the same as the name which get from pinned path.

The syscall information below show that map name "process_pinned_map"
is truncated to "process_pinned_".

    bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/process_pinned_map",
    bpf_fd=0, file_flags=0}, 144) = -1 ENOENT (No such file or directory)

    bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_HASH, key_size=4,
    value_size=4,max_entries=1024, map_flags=0, inner_map_fd=0,
    map_name="process_pinned_",map_ifindex=0, btf_fd=3, btf_key_type_id=6,
    btf_value_type_id=10,btf_vmlinux_value_type_id=0}, 72) = 4

This patch check that if the name of pinned map are the same as the
actual name for the first (BPF_OBJ_NAME_LEN - 1),
bpf map still uses the name which is included in bpf object.

Signed-off-by: Anquan Wu <leiqi96@hotmail.com>
---

v2: compare against zero explicitly

v1: https://lore.kernel.org/linux-kernel/OSZP286MB1725A2361FA2EE8432C4D5F4B8879@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
---
 tools/lib/bpf/libbpf.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e89cc9c885b3..7b4d3604dfb4 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4328,6 +4328,7 @@ int bpf_map__reuse_fd(struct bpf_map *map, int fd)
 {
 	struct bpf_map_info info = {};
 	__u32 len = sizeof(info);
+	__u32 name_len;
 	int new_fd, err;
 	char *new_name;
 
@@ -4337,7 +4338,12 @@ int bpf_map__reuse_fd(struct bpf_map *map, int fd)
 	if (err)
 		return libbpf_err(err);
 
-	new_name = strdup(info.name);
+	name_len = strlen(info.name);
+	if (name_len == BPF_OBJ_NAME_LEN - 1 && strncmp(map->name, info.name, name_len) == 0)
+		new_name = strdup(map->name);
+	else
+		new_name = strdup(info.name);
+
 	if (!new_name)
 		return libbpf_err(-errno);
 
-- 
2.32.0

Re: [PATCH v2] libbpf: fix the name of a reused map

[Jiri Olsa]

On Tue, Jul 12, 2022 at 11:15:40AM +0800, Anquan Wu wrote:
> BPF map name is limited to BPF_OBJ_NAME_LEN.
> A map name is defined as being longer than BPF_OBJ_NAME_LEN,
> it will be truncated to BPF_OBJ_NAME_LEN when a userspace program
> calls libbpf to create the map. A pinned map also generates a path
> in the /sys. If the previous program wanted to reuse the map，
> it can not get bpf_map by name, because the name of the map is only
> partially the same as the name which get from pinned path.
> 
> The syscall information below show that map name "process_pinned_map"
> is truncated to "process_pinned_".
> 
>     bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/process_pinned_map",
>     bpf_fd=0, file_flags=0}, 144) = -1 ENOENT (No such file or directory)
> 
>     bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_HASH, key_size=4,
>     value_size=4,max_entries=1024, map_flags=0, inner_map_fd=0,
>     map_name="process_pinned_",map_ifindex=0, btf_fd=3, btf_key_type_id=6,
>     btf_value_type_id=10,btf_vmlinux_value_type_id=0}, 72) = 4
> 
> This patch check that if the name of pinned map are the same as the
> actual name for the first (BPF_OBJ_NAME_LEN - 1),
> bpf map still uses the name which is included in bpf object.
> 
> Signed-off-by: Anquan Wu <leiqi96@hotmail.com>
> ---
> 
> v2: compare against zero explicitly
> 
> v1: https://lore.kernel.org/linux-kernel/OSZP286MB1725A2361FA2EE8432C4D5F4B8879@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
> ---
>  tools/lib/bpf/libbpf.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e89cc9c885b3..7b4d3604dfb4 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4328,6 +4328,7 @@ int bpf_map__reuse_fd(struct bpf_map *map, int fd)
>  {
>  	struct bpf_map_info info = {};
>  	__u32 len = sizeof(info);
> +	__u32 name_len;
>  	int new_fd, err;
>  	char *new_name;
>  
> @@ -4337,7 +4338,12 @@ int bpf_map__reuse_fd(struct bpf_map *map, int fd)
>  	if (err)
>  		return libbpf_err(err);
>  
> -	new_name = strdup(info.name);
> +	name_len = strlen(info.name);
> +	if (name_len == BPF_OBJ_NAME_LEN - 1 && strncmp(map->name, info.name, name_len) == 0)

so what if the map->name is different after 'name_len' ?

jirka

> +		new_name = strdup(map->name);
> +	else
> +		new_name = strdup(info.name);
> +
>  	if (!new_name)
>  		return libbpf_err(-errno);
>  
> -- 
> 2.32.0
> 

Re: [PATCH v2] libbpf: fix the name of a reused map

[Anquan Wu]

On Tue, 2022-07-12 at 10:31 +0200, Jiri Olsa wrote:
> On Tue, Jul 12, 2022 at 11:15:40AM +0800, Anquan Wu wrote:
> > BPF map name is limited to BPF_OBJ_NAME_LEN.
> > A map name is defined as being longer than BPF_OBJ_NAME_LEN,
> > it will be truncated to BPF_OBJ_NAME_LEN when a userspace program
> > calls libbpf to create the map. A pinned map also generates a path
> > in the /sys. If the previous program wanted to reuse the map，
> > it can not get bpf_map by name, because the name of the map is only
> > partially the same as the name which get from pinned path.
> > 
> > The syscall information below show that map name "process_pinned_map"
> > is truncated to "process_pinned_".
> > 
> >     bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/process_pinned_map",
> >     bpf_fd=0, file_flags=0}, 144) = -1 ENOENT (No such file or
> > directory)
> > 
> >     bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_HASH, key_size=4,
> >     value_size=4,max_entries=1024, map_flags=0, inner_map_fd=0,
> >     map_name="process_pinned_",map_ifindex=0, btf_fd=3,
> > btf_key_type_id=6,
> >     btf_value_type_id=10,btf_vmlinux_value_type_id=0}, 72) = 4
> > 
> > This patch check that if the name of pinned map are the same as the
> > actual name for the first (BPF_OBJ_NAME_LEN - 1),
> > bpf map still uses the name which is included in bpf object.
> > 
> > Signed-off-by: Anquan Wu <leiqi96@hotmail.com>
> > ---
> > 
> > v2: compare against zero explicitly
> > 
> > v1:
> > https://lore.kernel.org/linux-kernel/OSZP286MB1725A2361FA2EE8432C4D5F4B8879@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
> > ---
> >  tools/lib/bpf/libbpf.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e89cc9c885b3..7b4d3604dfb4 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4328,6 +4328,7 @@ int bpf_map__reuse_fd(struct bpf_map *map, int
> > fd)
> >  {
> >         struct bpf_map_info info = {};
> >         __u32 len = sizeof(info);
> > +       __u32 name_len;
> >         int new_fd, err;
> >         char *new_name;
> >  
> > @@ -4337,7 +4338,12 @@ int bpf_map__reuse_fd(struct bpf_map *map, int
> > fd)
> >         if (err)
> >                 return libbpf_err(err);
> >  
> > -       new_name = strdup(info.name);
> > +       name_len = strlen(info.name);
> > +       if (name_len == BPF_OBJ_NAME_LEN - 1 && strncmp(map->name,
> > info.name, name_len) == 0)
> 
> so what if the map->name is different after 'name_len' ?
> 
> jirka
> 

If  A map name is defined as being longer than name_len (name_len is 
"BPF_OBJ_NAME_LEN - 1" in this context), a program will fail to get a
reused bpf_map by bpf_object__find_map_by_name().
   
   fromhttps://github.com/libbpf/libbpf/blob/master/src/libbpf.c#L9295,
   pos->name in bpf_object__find_map_by_name() is from  new_name in      
   bpf_map_reuse_fd(). It can not find map by the name which is defined
   in bpf object.

I wrote some code to verify this problem and test the solution
mentioned above.
Link: https://github.com/leiqi96/libbpf-fix

Anquan


> > +               new_name = strdup(map->name);
> > +       else
> > +               new_name = strdup(info.name);
> > +
> >         if (!new_name)
> >                 return libbpf_err(-errno);
> >  
> > -- 
> > 2.32.0
> > 

Re: [PATCH v2] libbpf: fix the name of a reused map

[Anquan Wu]

On Tue, 2022-07-12 at 10:31 +0200, Jiri Olsa wrote:
> On Tue, Jul 12, 2022 at 11:15:40AM +0800, Anquan Wu wrote:
> > BPF map name is limited to BPF_OBJ_NAME_LEN.
> > A map name is defined as being longer than BPF_OBJ_NAME_LEN,
> > it will be truncated to BPF_OBJ_NAME_LEN when a userspace program
> > calls libbpf to create the map. A pinned map also generates a path
> > in the /sys. If the previous program wanted to reuse the map，
> > it can not get bpf_map by name, because the name of the map is only
> > partially the same as the name which get from pinned path.
> > 
> > The syscall information below show that map name
> > "process_pinned_map"
> > is truncated to "process_pinned_".
> > 
> >     bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/process_pinned_map",
> >     bpf_fd=0, file_flags=0}, 144) = -1 ENOENT (No such file or
> > directory)
> > 
> >     bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_HASH, key_size=4,
> >     value_size=4,max_entries=1024, map_flags=0, inner_map_fd=0,
> >     map_name="process_pinned_",map_ifindex=0, btf_fd=3,
> > btf_key_type_id=6,
> >     btf_value_type_id=10,btf_vmlinux_value_type_id=0}, 72) = 4
> > 
> > This patch check that if the name of pinned map are the same as the
> > actual name for the first (BPF_OBJ_NAME_LEN - 1),
> > bpf map still uses the name which is included in bpf object.
> > 
> > Signed-off-by: Anquan Wu <leiqi96@hotmail.com>
> > ---
> > 
> > v2: compare against zero explicitly
> > 
> > v1:
> > https://lore.kernel.org/linux-kernel/OSZP286MB1725A2361FA2EE8432C4D5F4B8879@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
> > ---
> >  tools/lib/bpf/libbpf.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e89cc9c885b3..7b4d3604dfb4 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4328,6 +4328,7 @@ int bpf_map__reuse_fd(struct bpf_map *map,
> > int
> > fd)
> >  {
> >         struct bpf_map_info info = {};
> >         __u32 len = sizeof(info);
> > +       __u32 name_len;
> >         int new_fd, err;
> >         char *new_name;
> >  
> > @@ -4337,7 +4338,12 @@ int bpf_map__reuse_fd(struct bpf_map *map,
> > int
> > fd)
> >         if (err)
> >                 return libbpf_err(err);
> >  
> > -       new_name = strdup(info.name);
> > +       name_len = strlen(info.name);
> > +       if (name_len == BPF_OBJ_NAME_LEN - 1 && strncmp(map->name,
> > info.name, name_len) == 0)
> 
> so what if the map->name is different after 'name_len' ?
> 
> jirka
> 

If  A map name is defined as being longer than name_len (name_len is 
"BPF_OBJ_NAME_LEN - 1" in this context), a program will fail to get a
reused bpf_map by bpf_object__find_map_by_name().
   
   fromhttps://github.com/libbpf/libbpf/blob/master/src/libbpf.c#L9295,
   pos->name in bpf_object__find_map_by_name() is from  new_name
in      
   bpf_map_reuse_fd(). It can not find map by the name which is defined
   in bpf object.

I wrote some code to verify this problem and test the solution
mentioned above.
Link: https://github.com/leiqi96/libbpf-fix

Anquan


> > +               new_name = strdup(map->name);
> > +       else
> > +               new_name = strdup(info.name);
> > +
> >         if (!new_name)
> >                 return libbpf_err(-errno);
> >  
> > -- 
> > 2.32.0
> > 

Re: [PATCH v2] libbpf: fix the name of a reused map

[Andrii Nakryiko]

On Tue, Jul 12, 2022 at 10:59 PM Anquan Wu <leiqi96@hotmail.com> wrote:
>
> On Tue, 2022-07-12 at 10:31 +0200, Jiri Olsa wrote:
> > On Tue, Jul 12, 2022 at 11:15:40AM +0800, Anquan Wu wrote:
> > > BPF map name is limited to BPF_OBJ_NAME_LEN.
> > > A map name is defined as being longer than BPF_OBJ_NAME_LEN,
> > > it will be truncated to BPF_OBJ_NAME_LEN when a userspace program
> > > calls libbpf to create the map. A pinned map also generates a path
> > > in the /sys. If the previous program wanted to reuse the map，
> > > it can not get bpf_map by name, because the name of the map is only
> > > partially the same as the name which get from pinned path.
> > >
> > > The syscall information below show that map name
> > > "process_pinned_map"
> > > is truncated to "process_pinned_".
> > >
> > >     bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/process_pinned_map",
> > >     bpf_fd=0, file_flags=0}, 144) = -1 ENOENT (No such file or
> > > directory)
> > >
> > >     bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_HASH, key_size=4,
> > >     value_size=4,max_entries=1024, map_flags=0, inner_map_fd=0,
> > >     map_name="process_pinned_",map_ifindex=0, btf_fd=3,
> > > btf_key_type_id=6,
> > >     btf_value_type_id=10,btf_vmlinux_value_type_id=0}, 72) = 4
> > >
> > > This patch check that if the name of pinned map are the same as the
> > > actual name for the first (BPF_OBJ_NAME_LEN - 1),
> > > bpf map still uses the name which is included in bpf object.
> > >
> > > Signed-off-by: Anquan Wu <leiqi96@hotmail.com>
> > > ---
> > >
> > > v2: compare against zero explicitly
> > >
> > > v1:
> > > https://lore.kernel.org/linux-kernel/OSZP286MB1725A2361FA2EE8432C4D5F4B8879@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
> > > ---
> > >  tools/lib/bpf/libbpf.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index e89cc9c885b3..7b4d3604dfb4 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -4328,6 +4328,7 @@ int bpf_map__reuse_fd(struct bpf_map *map,
> > > int
> > > fd)
> > >  {
> > >         struct bpf_map_info info = {};
> > >         __u32 len = sizeof(info);
> > > +       __u32 name_len;
> > >         int new_fd, err;
> > >         char *new_name;
> > >
> > > @@ -4337,7 +4338,12 @@ int bpf_map__reuse_fd(struct bpf_map *map,
> > > int
> > > fd)
> > >         if (err)
> > >                 return libbpf_err(err);
> > >
> > > -       new_name = strdup(info.name);
> > > +       name_len = strlen(info.name);
> > > +       if (name_len == BPF_OBJ_NAME_LEN - 1 && strncmp(map->name,
> > > info.name, name_len) == 0)
> >
> > so what if the map->name is different after 'name_len' ?
> >
> > jirka
> >
>
> If  A map name is defined as being longer than name_len (name_len is
> "BPF_OBJ_NAME_LEN - 1" in this context), a program will fail to get a
> reused bpf_map by bpf_object__find_map_by_name().
>
>    fromhttps://github.com/libbpf/libbpf/blob/master/src/libbpf.c#L9295,
>    pos->name in bpf_object__find_map_by_name() is from  new_name
> in
>    bpf_map_reuse_fd(). It can not find map by the name which is defined
>    in bpf object.
>
> I wrote some code to verify this problem and test the solution
> mentioned above.
> Link: https://github.com/leiqi96/libbpf-fix
>

It would be great to have something like this as a selftest, please
send a follow up patch adding a test under selftests/bpf for map
reuse. See prog_tests/pinning.c, this might belong there.

To also answer Jiri's question. This is not an ideal solution, but it
improves the current situation. And while potentially it's not 100%
correct (because only checks first 15 characters), user normally would
use bpf_map__reuse_fd() on well-known and presumably correct map, so
chance of misuse here are pretty minimal.

So I added

Fixes: 26736eb9a483 ("tools: libbpf: allow map reuse")

and applied to bpf-next, thanks.

> Anquan
>
>
> > > +               new_name = strdup(map->name);
> > > +       else
> > > +               new_name = strdup(info.name);
> > > +
> > >         if (!new_name)
> > >                 return libbpf_err(-errno);
> > >
> > > --
> > > 2.32.0
> > >
>
>
>

Re: [PATCH v2] libbpf: fix the name of a reused map

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Tue, 12 Jul 2022 11:15:40 +0800 you wrote:
> BPF map name is limited to BPF_OBJ_NAME_LEN.
> A map name is defined as being longer than BPF_OBJ_NAME_LEN,
> it will be truncated to BPF_OBJ_NAME_LEN when a userspace program
> calls libbpf to create the map. A pinned map also generates a path
> in the /sys. If the previous program wanted to reuse the map，
> it can not get bpf_map by name, because the name of the map is only
> partially the same as the name which get from pinned path.
> 
> [...]

Here is the summary with links:
  - [v2] libbpf: fix the name of a reused map
    https://git.kernel.org/bpf/bpf-next/c/bf3f00378524

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH v2] libbpf: fix the name of a reused map

[Anquan Wu]

[PATCH v2] libbpf: fix the name of a reused map

[Anquan Wu]