[PATCH v2 bpf-next] libbpf: make remark about zero-initializing bpf_*_info structs

[PATCH v2 bpf-next] libbpf: make remark about zero-initializing bpf_*_info structs

[Matt Bobrowski]

In some situations, if you fail to zero-initialize the
bpf_{prog,map,btf,link}_info structs supplied to the set of LIBBPF
helpers bpf_{prog,map,btf,link}_get_info_by_fd(), you can expect the
helper to return an error. This can possibly leave people in a
situation where they're scratching their heads for an unnnecessary
amount of time. Make an explicit remark about the requirement of
zero-initializing the supplied bpf_{prog,map,btf,link}_info structs
for the respective LIBBPF helpers.

Internally, LIBBPF helpers bpf_{prog,map,btf,link}_get_info_by_fd()
call into bpf_obj_get_info_by_fd() where the bpf(2)
BPF_OBJ_GET_INFO_BY_FD command is used. This specific command is
effectively backed by restrictions enforced by the
bpf_check_uarg_tail_zero() helper. This function ensures that if the
size of the supplied bpf_{prog,map,btf,link}_info structs are larger
than what the kernel can handle, trailing bits are zeroed. This can be
a problem when compiling against UAPI headers that don't necessarily
match the sizes of the same underlying types known to the kernel.

Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 tools/lib/bpf/bpf.h | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index f866e98b2436..3ed745f99da3 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -500,7 +500,10 @@ LIBBPF_API int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len);
  * program corresponding to *prog_fd*.
  *
  * Populates up to *info_len* bytes of *info* and updates *info_len* with the
- * actual number of bytes written to *info*.
+ * actual number of bytes written to *info*. Note that *info* should be
+ * zero-initialized before calling into this helper. Failing to zero-initialize
+ * *info* under certain circumstances can result in this helper returning an
+ * error.
  *
  * @param prog_fd BPF program file descriptor
  * @param info pointer to **struct bpf_prog_info** that will be populated with
@@ -517,7 +520,10 @@ LIBBPF_API int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
  * map corresponding to *map_fd*.
  *
  * Populates up to *info_len* bytes of *info* and updates *info_len* with the
- * actual number of bytes written to *info*.
+ * actual number of bytes written to *info*. Note that *info* should be
+ * zero-initialized before calling into this helper. Failing to zero-initialize
+ * *info* under certain circumstances can result in this helper returning an
+ * error.
  *
  * @param map_fd BPF map file descriptor
  * @param info pointer to **struct bpf_map_info** that will be populated with
@@ -530,11 +536,14 @@ LIBBPF_API int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
 LIBBPF_API int bpf_map_get_info_by_fd(int map_fd, struct bpf_map_info *info, __u32 *info_len);
 
 /**
- * @brief **bpf_btf_get_info_by_fd()** obtains information about the 
+ * @brief **bpf_btf_get_info_by_fd()** obtains information about the
  * BTF object corresponding to *btf_fd*.
  *
  * Populates up to *info_len* bytes of *info* and updates *info_len* with the
- * actual number of bytes written to *info*.
+ * actual number of bytes written to *info*. Note that *info* should be
+ * zero-initialized before calling into this helper. Failing to zero-initialize
+ * *info* under certain circumstances can result in this helper returning an
+ * error.
  *
  * @param btf_fd BTF object file descriptor
  * @param info pointer to **struct bpf_btf_info** that will be populated with
@@ -551,7 +560,10 @@ LIBBPF_API int bpf_btf_get_info_by_fd(int btf_fd, struct bpf_btf_info *info, __u
  * link corresponding to *link_fd*.
  *
  * Populates up to *info_len* bytes of *info* and updates *info_len* with the
- * actual number of bytes written to *info*.
+ * actual number of bytes written to *info*. Note that *info* should be
+ * zero-initialized before calling into this helper. Failing to zero-initialize
+ * *info* under certain circumstances can result in this helper returning an
+ * error.
  *
  * @param link_fd BPF link file descriptor
  * @param info pointer to **struct bpf_link_info** that will be populated with
-- 
2.43.0.687.g38aa6559b0-goog

/M

Re: [PATCH v2 bpf-next] libbpf: make remark about zero-initializing bpf_*_info structs

[Jiri Olsa]

On Wed, Feb 14, 2024 at 07:30:25AM +0000, Matt Bobrowski wrote:
> In some situations, if you fail to zero-initialize the
> bpf_{prog,map,btf,link}_info structs supplied to the set of LIBBPF
> helpers bpf_{prog,map,btf,link}_get_info_by_fd(), you can expect the
> helper to return an error. This can possibly leave people in a
> situation where they're scratching their heads for an unnnecessary
> amount of time. Make an explicit remark about the requirement of
> zero-initializing the supplied bpf_{prog,map,btf,link}_info structs
> for the respective LIBBPF helpers.
> 
> Internally, LIBBPF helpers bpf_{prog,map,btf,link}_get_info_by_fd()
> call into bpf_obj_get_info_by_fd() where the bpf(2)
> BPF_OBJ_GET_INFO_BY_FD command is used. This specific command is
> effectively backed by restrictions enforced by the
> bpf_check_uarg_tail_zero() helper. This function ensures that if the
> size of the supplied bpf_{prog,map,btf,link}_info structs are larger
> than what the kernel can handle, trailing bits are zeroed. This can be
> a problem when compiling against UAPI headers that don't necessarily
> match the sizes of the same underlying types known to the kernel.
> 
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
>  tools/lib/bpf/bpf.h | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index f866e98b2436..3ed745f99da3 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -500,7 +500,10 @@ LIBBPF_API int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len);
>   * program corresponding to *prog_fd*.
>   *
>   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> - * actual number of bytes written to *info*.
> + * actual number of bytes written to *info*. Note that *info* should be
> + * zero-initialized before calling into this helper. Failing to zero-initialize
> + * *info* under certain circumstances can result in this helper returning an
> + * error.
>   *
>   * @param prog_fd BPF program file descriptor
>   * @param info pointer to **struct bpf_prog_info** that will be populated with
> @@ -517,7 +520,10 @@ LIBBPF_API int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
>   * map corresponding to *map_fd*.
>   *
>   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> - * actual number of bytes written to *info*.
> + * actual number of bytes written to *info*. Note that *info* should be
> + * zero-initialized before calling into this helper. Failing to zero-initialize
> + * *info* under certain circumstances can result in this helper returning an
> + * error.
>   *
>   * @param map_fd BPF map file descriptor
>   * @param info pointer to **struct bpf_map_info** that will be populated with
> @@ -530,11 +536,14 @@ LIBBPF_API int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
>  LIBBPF_API int bpf_map_get_info_by_fd(int map_fd, struct bpf_map_info *info, __u32 *info_len);
>  
>  /**
> - * @brief **bpf_btf_get_info_by_fd()** obtains information about the 
> + * @brief **bpf_btf_get_info_by_fd()** obtains information about the
>   * BTF object corresponding to *btf_fd*.
>   *
>   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> - * actual number of bytes written to *info*.
> + * actual number of bytes written to *info*. Note that *info* should be
> + * zero-initialized before calling into this helper. Failing to zero-initialize
> + * *info* under certain circumstances can result in this helper returning an
> + * error.
>   *
>   * @param btf_fd BTF object file descriptor
>   * @param info pointer to **struct bpf_btf_info** that will be populated with
> @@ -551,7 +560,10 @@ LIBBPF_API int bpf_btf_get_info_by_fd(int btf_fd, struct bpf_btf_info *info, __u
>   * link corresponding to *link_fd*.
>   *
>   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> - * actual number of bytes written to *info*.
> + * actual number of bytes written to *info*. Note that *info* should be
> + * zero-initialized before calling into this helper. Failing to zero-initialize
> + * *info* under certain circumstances can result in this helper returning an
> + * error.

this is slightly misleading, because like for uprobe/kprobe multi links we normally
call bpf_link_get_info_by_fd twice, first time with zero initialed info to get the
static data and then again with info filled with user space buffer pointers to get
other data like addresses or cookies.. I think to some extend this is similar also
for bpf_prog_get_info_by_fd

maybe something like:

Note that *info* should be zero-initialized or initialized as expected by the
requested object type. Failing to (zero)initialize *info* under certain circumstances
can result in this helper returning an error.

jirka

>   *
>   * @param link_fd BPF link file descriptor
>   * @param info pointer to **struct bpf_link_info** that will be populated with
> -- 
> 2.43.0.687.g38aa6559b0-goog
> 
> /M
> 

Re: [PATCH v2 bpf-next] libbpf: make remark about zero-initializing bpf_*_info structs

[Matt Bobrowski]

On Wed, Feb 14, 2024 at 09:23:53AM +0100, Jiri Olsa wrote:
> On Wed, Feb 14, 2024 at 07:30:25AM +0000, Matt Bobrowski wrote:
> > In some situations, if you fail to zero-initialize the
> > bpf_{prog,map,btf,link}_info structs supplied to the set of LIBBPF
> > helpers bpf_{prog,map,btf,link}_get_info_by_fd(), you can expect the
> > helper to return an error. This can possibly leave people in a
> > situation where they're scratching their heads for an unnnecessary
> > amount of time. Make an explicit remark about the requirement of
> > zero-initializing the supplied bpf_{prog,map,btf,link}_info structs
> > for the respective LIBBPF helpers.
> > 
> > Internally, LIBBPF helpers bpf_{prog,map,btf,link}_get_info_by_fd()
> > call into bpf_obj_get_info_by_fd() where the bpf(2)
> > BPF_OBJ_GET_INFO_BY_FD command is used. This specific command is
> > effectively backed by restrictions enforced by the
> > bpf_check_uarg_tail_zero() helper. This function ensures that if the
> > size of the supplied bpf_{prog,map,btf,link}_info structs are larger
> > than what the kernel can handle, trailing bits are zeroed. This can be
> > a problem when compiling against UAPI headers that don't necessarily
> > match the sizes of the same underlying types known to the kernel.
> > 
> > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> > ---
> >  tools/lib/bpf/bpf.h | 22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index f866e98b2436..3ed745f99da3 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -500,7 +500,10 @@ LIBBPF_API int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len);
> >   * program corresponding to *prog_fd*.
> >   *
> >   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> > - * actual number of bytes written to *info*.
> > + * actual number of bytes written to *info*. Note that *info* should be
> > + * zero-initialized before calling into this helper. Failing to zero-initialize
> > + * *info* under certain circumstances can result in this helper returning an
> > + * error.
> >   *
> >   * @param prog_fd BPF program file descriptor
> >   * @param info pointer to **struct bpf_prog_info** that will be populated with
> > @@ -517,7 +520,10 @@ LIBBPF_API int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
> >   * map corresponding to *map_fd*.
> >   *
> >   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> > - * actual number of bytes written to *info*.
> > + * actual number of bytes written to *info*. Note that *info* should be
> > + * zero-initialized before calling into this helper. Failing to zero-initialize
> > + * *info* under certain circumstances can result in this helper returning an
> > + * error.
> >   *
> >   * @param map_fd BPF map file descriptor
> >   * @param info pointer to **struct bpf_map_info** that will be populated with
> > @@ -530,11 +536,14 @@ LIBBPF_API int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
> >  LIBBPF_API int bpf_map_get_info_by_fd(int map_fd, struct bpf_map_info *info, __u32 *info_len);
> >  
> >  /**
> > - * @brief **bpf_btf_get_info_by_fd()** obtains information about the 
> > + * @brief **bpf_btf_get_info_by_fd()** obtains information about the
> >   * BTF object corresponding to *btf_fd*.
> >   *
> >   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> > - * actual number of bytes written to *info*.
> > + * actual number of bytes written to *info*. Note that *info* should be
> > + * zero-initialized before calling into this helper. Failing to zero-initialize
> > + * *info* under certain circumstances can result in this helper returning an
> > + * error.
> >   *
> >   * @param btf_fd BTF object file descriptor
> >   * @param info pointer to **struct bpf_btf_info** that will be populated with
> > @@ -551,7 +560,10 @@ LIBBPF_API int bpf_btf_get_info_by_fd(int btf_fd, struct bpf_btf_info *info, __u
> >   * link corresponding to *link_fd*.
> >   *
> >   * Populates up to *info_len* bytes of *info* and updates *info_len* with the
> > - * actual number of bytes written to *info*.
> > + * actual number of bytes written to *info*. Note that *info* should be
> > + * zero-initialized before calling into this helper. Failing to zero-initialize
> > + * *info* under certain circumstances can result in this helper returning an
> > + * error.
> 
> this is slightly misleading, because like for uprobe/kprobe multi links we normally
> call bpf_link_get_info_by_fd twice, first time with zero initialed info to get the
> static data and then again with info filled with user space buffer pointers to get
> other data like addresses or cookies.. I think to some extend this is similar also
> for bpf_prog_get_info_by_fd

Ah, you're right, it is slightly misleading as the current wording
implies that the supplied buffer must always be zeroed out, when in
reality that's not true.

> maybe something like:
> 
> Note that *info* should be zero-initialized or initialized as expected by the
> requested object type. Failing to (zero)initialize *info* under certain circumstances
> can result in this helper returning an error.

I'll adapt as per your recommendations and then resend this through
once again.

Thanks for the feedback!

/M