[PATCH bpf 0/2] tools: bpftool: fix unlikely race and JSON output on error path

[PATCH bpf 0/2] tools: bpftool: fix unlikely race and JSON output on error path

[Jakub Kicinski]

Hi!

Two small fixes here to listing maps and programs.  The loop for showing
maps is written slightly differently to programs which was missed in JSON
output support, and output would be broken if any of the system calls
failed.  Second fix is in very unlikely case that program or map disappears
after we get its ID we should just skip over that object instead of failing.

Jakub Kicinski (2):
  tools: bpftool: maps: close json array on error paths of show
  tools: bpftool: protect against races with disappearing objects

 tools/bpf/bpftool/map.c  | 8 +++++---
 tools/bpf/bpftool/prog.c | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.15.1

[PATCH bpf 1/2] tools: bpftool: maps: close json array on error paths of show

[Jakub Kicinski]

We can't return from the middle of do_show(), because
json_array will not be closed.  Break out of the loop.
Note that the error handling after the loop depends on
errno, so no need to set err.

Fixes: 831a0aafe5c3 ("tools: bpftool: add JSON output for `bpftool map *` commands")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/map.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index e2450c8e88e6..8368b7ea31b5 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -523,21 +523,21 @@ static int do_show(int argc, char **argv)
 				break;
 			p_err("can't get next map: %s%s", strerror(errno),
 			      errno == EINVAL ? " -- kernel too old?" : "");
-			return -1;
+			break;
 		}
 
 		fd = bpf_map_get_fd_by_id(id);
 		if (fd < 0) {
 			p_err("can't get map by id (%u): %s",
 			      id, strerror(errno));
-			return -1;
+			break;
 		}
 
 		err = bpf_obj_get_info_by_fd(fd, &info, &len);
 		if (err) {
 			p_err("can't get map info: %s", strerror(errno));
 			close(fd);
-			return -1;
+			break;
 		}
 
 		if (json_output)
-- 
2.15.1

[PATCH bpf 2/2] tools: bpftool: protect against races with disappearing objects

[Jakub Kicinski]

On program/map show we may get an ID of an object from GETNEXT,
but the object may disappear before we call GET_FD_BY_ID.  If
that happens, ignore the object and continue.

Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/map.c  | 2 ++
 tools/bpf/bpftool/prog.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 8368b7ea31b5..a8c3a33dd185 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -528,6 +528,8 @@ static int do_show(int argc, char **argv)
 
 		fd = bpf_map_get_fd_by_id(id);
 		if (fd < 0) {
+			if (errno == ENOENT)
+				continue;
 			p_err("can't get map by id (%u): %s",
 			      id, strerror(errno));
 			break;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index ad619b96c276..dded77345bfb 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -382,6 +382,8 @@ static int do_show(int argc, char **argv)
 
 		fd = bpf_prog_get_fd_by_id(id);
 		if (fd < 0) {
+			if (errno == ENOENT)
+				continue;
 			p_err("can't get prog by id (%u): %s",
 			      id, strerror(errno));
 			err = -1;
-- 
2.15.1

Re: [PATCH bpf 0/2] tools: bpftool: fix unlikely race and JSON output on error path

[Daniel Borkmann]

On 12/22/2017 08:36 PM, Jakub Kicinski wrote:
> Hi!
> 
> Two small fixes here to listing maps and programs.  The loop for showing
> maps is written slightly differently to programs which was missed in JSON
> output support, and output would be broken if any of the system calls
> failed.  Second fix is in very unlikely case that program or map disappears
> after we get its ID we should just skip over that object instead of failing.

Applied to bpf tree, thanks Jakub!