* [PATCH bpf] libbpf: always NULL out pobj in bpf_prog_load_xattr
@ 2019-05-02 15:49 Lorenz Bauer
2019-05-02 16:03 ` Martin Lau
2019-05-08 16:49 ` [PATCH bpf v2] selftests: bpf: initialize bpf_object pointers where needed Lorenz Bauer
0 siblings, 2 replies; 6+ messages in thread
From: Lorenz Bauer @ 2019-05-02 15:49 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: Lorenz Bauer
Currently, code like the following segfaults if bpf_prog_load_xattr
returns an error:
struct bpf_object *obj;
err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);
bpf_object__close(obj);
if (err)
...
Unconditionally reset pobj to NULL at the start of the function
to fix this.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
tools/lib/bpf/libbpf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 11a65db4b93f..2ddf3212b8f7 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3363,6 +3363,8 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
struct bpf_map *map;
int err;
+ *pobj = NULL;
+
if (!attr)
return -EINVAL;
if (!attr->file)
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] libbpf: always NULL out pobj in bpf_prog_load_xattr
2019-05-02 15:49 [PATCH bpf] libbpf: always NULL out pobj in bpf_prog_load_xattr Lorenz Bauer
@ 2019-05-02 16:03 ` Martin Lau
2019-05-07 9:09 ` Lorenz Bauer
2019-05-08 16:49 ` [PATCH bpf v2] selftests: bpf: initialize bpf_object pointers where needed Lorenz Bauer
1 sibling, 1 reply; 6+ messages in thread
From: Martin Lau @ 2019-05-02 16:03 UTC (permalink / raw)
To: Lorenz Bauer; +Cc: bpf, netdev, ast, daniel
On Thu, May 02, 2019 at 11:49:32AM -0400, Lorenz Bauer wrote:
> Currently, code like the following segfaults if bpf_prog_load_xattr
> returns an error:
>
> struct bpf_object *obj;
>
> err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);
> bpf_object__close(obj);
This is a bug. err should always be checked first before
using obj or prog_fd. If there is err, calling bpf_object__close(NULL)
is another redundancy.
> if (err)
> ...
>
> Unconditionally reset pobj to NULL at the start of the function
> to fix this.
Hence, this change is unnecessary.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
> tools/lib/bpf/libbpf.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 11a65db4b93f..2ddf3212b8f7 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3363,6 +3363,8 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> struct bpf_map *map;
> int err;
>
> + *pobj = NULL;
> +
> if (!attr)
> return -EINVAL;
> if (!attr->file)
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] libbpf: always NULL out pobj in bpf_prog_load_xattr
2019-05-02 16:03 ` Martin Lau
@ 2019-05-07 9:09 ` Lorenz Bauer
0 siblings, 0 replies; 6+ messages in thread
From: Lorenz Bauer @ 2019-05-07 9:09 UTC (permalink / raw)
To: Martin Lau; +Cc: bpf, netdev, ast, daniel
On Thu, 2 May 2019 at 17:03, Martin Lau <kafai@fb.com> wrote:
>
> On Thu, May 02, 2019 at 11:49:32AM -0400, Lorenz Bauer wrote:
> > Currently, code like the following segfaults if bpf_prog_load_xattr
> > returns an error:
> >
> > struct bpf_object *obj;
> >
> > err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);
> > bpf_object__close(obj);
> This is a bug. err should always be checked first before
> using obj or prog_fd. If there is err, calling bpf_object__close(NULL)
> is another redundancy.
Ack, I'll send another patch. The bug is in a couple of places, so I
figured that this is something people do naturally.
>
> > if (err)
> > ...
> >
> > Unconditionally reset pobj to NULL at the start of the function
> > to fix this.
> Hence, this change is unnecessary.
>
> >
> > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> > ---
> > tools/lib/bpf/libbpf.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 11a65db4b93f..2ddf3212b8f7 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3363,6 +3363,8 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> > struct bpf_map *map;
> > int err;
> >
> > + *pobj = NULL;
> > +
> > if (!attr)
> > return -EINVAL;
> > if (!attr->file)
> > --
> > 2.19.1
> >
--
Lorenz Bauer | Systems Engineer
25 Lavington St., London SE1 0NZ
www.cloudflare.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf v2] selftests: bpf: initialize bpf_object pointers where needed
2019-05-02 15:49 [PATCH bpf] libbpf: always NULL out pobj in bpf_prog_load_xattr Lorenz Bauer
2019-05-02 16:03 ` Martin Lau
@ 2019-05-08 16:49 ` Lorenz Bauer
2019-05-09 15:56 ` Martin Lau
1 sibling, 1 reply; 6+ messages in thread
From: Lorenz Bauer @ 2019-05-08 16:49 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, kafai; +Cc: Lorenz Bauer
There are a few tests which call bpf_object__close on uninitialized
bpf_object*, which may segfault. Explicitly zero-initialise these pointers
to avoid this.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c | 2 +-
tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c | 2 +-
tools/testing/selftests/bpf/prog_tests/tp_attach_query.c | 3 +++
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index 23b159d95c3f..b74e2f6e96d0 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -15,7 +15,7 @@ static int libbpf_debug_print(enum libbpf_print_level level,
static int check_load(const char *file)
{
struct bpf_prog_load_attr attr;
- struct bpf_object *obj;
+ struct bpf_object *obj = NULL;
int err, prog_fd;
memset(&attr, 0, sizeof(struct bpf_prog_load_attr));
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
index d636a4f39476..f9b70e81682b 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
@@ -9,7 +9,7 @@ static void test_task_fd_query_tp_core(const char *probe_name,
struct perf_event_attr attr = {};
__u64 probe_offset, probe_addr;
__u32 len, prog_id, fd_type;
- struct bpf_object *obj;
+ struct bpf_object *obj = NULL;
__u32 duration = 0;
char buf[256];
diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
index a2f476f91637..fb095e5cd9af 100644
--- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
+++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
@@ -13,6 +13,9 @@ void test_tp_attach_query(void)
struct bpf_prog_info prog_info;
char buf[256];
+ for (i = 0; i < num_progs; i++)
+ obj[i] = NULL;
+
snprintf(buf, sizeof(buf),
"/sys/kernel/debug/tracing/events/sched/sched_switch/id");
efd = open(buf, O_RDONLY, 0);
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2] selftests: bpf: initialize bpf_object pointers where needed
2019-05-08 16:49 ` [PATCH bpf v2] selftests: bpf: initialize bpf_object pointers where needed Lorenz Bauer
@ 2019-05-09 15:56 ` Martin Lau
2019-05-09 23:01 ` Alexei Starovoitov
0 siblings, 1 reply; 6+ messages in thread
From: Martin Lau @ 2019-05-09 15:56 UTC (permalink / raw)
Cc: bpf, netdev, ast, daniel
On Wed, May 08, 2019 at 05:49:32PM +0100, Lorenz Bauer wrote:
> There are a few tests which call bpf_object__close on uninitialized
> bpf_object*, which may segfault. Explicitly zero-initialise these pointers
> to avoid this.
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2] selftests: bpf: initialize bpf_object pointers where needed
2019-05-09 15:56 ` Martin Lau
@ 2019-05-09 23:01 ` Alexei Starovoitov
0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2019-05-09 23:01 UTC (permalink / raw)
To: Martin Lau; +Cc: bpf, netdev, ast, daniel
On Thu, May 9, 2019 at 8:56 AM Martin Lau <kafai@fb.com> wrote:
>
> On Wed, May 08, 2019 at 05:49:32PM +0100, Lorenz Bauer wrote:
> > There are a few tests which call bpf_object__close on uninitialized
> > bpf_object*, which may segfault. Explicitly zero-initialise these pointers
> > to avoid this.
> Acked-by: Martin KaFai Lau <kafai@fb.com>
Applied. Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-05-09 23:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-02 15:49 [PATCH bpf] libbpf: always NULL out pobj in bpf_prog_load_xattr Lorenz Bauer
2019-05-02 16:03 ` Martin Lau
2019-05-07 9:09 ` Lorenz Bauer
2019-05-08 16:49 ` [PATCH bpf v2] selftests: bpf: initialize bpf_object pointers where needed Lorenz Bauer
2019-05-09 15:56 ` Martin Lau
2019-05-09 23:01 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).