On Mon, Jan 28, 2019 at 10:06 AM Jakub Kicinski
<jakub.kicin...@netronome.com> wrote:
>
> Prog arrays don't have 'owner_prog_type' and 'owner_jited'
> fields in their fdinfo when they are created. Those fields
> are set and reported when first program is checked for
> compatibility by bpf_prog_array_compatible().
>
> This means that bpftool cannot expect the fields to always
> be there. Currently trying to show maps on a system with
> an un-owned prog array leads to a crash:
>
> $ bpftool map show
> 389: prog_array name tail_call_map flags 0x0
> Error: key 'owner_prog_type' not found in fdinfo
> Error: key 'owner_jited' not found in fdinfo
> key 4B value 4B max_entries 4 memlock 4096B
> Segmentation fault (core dumped)
>
> We pass a NULL pointer to atoi().
>
> Remove the assumption that fdinfo keys are always present.
> Add missing validations and remove the p_err() calls which
> may lead to broken JSON output as caller will not propagate
> the failure.
>
> Fixes: 99a44bef5870 ("tools: bpftool: add owner_prog_type and owner_jited to
> bpftool output")
> Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.mon...@netronome.com>
Acked-by: Song Liu <songliubrav...@fb.com>
> ---
> tools/bpf/bpftool/common.c | 6 +-----
> tools/bpf/bpftool/map.c | 17 ++++++++---------
> 2 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index 897483457bf0..f7261fad45c1 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -297,10 +297,8 @@ char *get_fdinfo(int fd, const char *key)
> snprintf(path, sizeof(path), "/proc/self/fdinfo/%d", fd);
>
> fdi = fopen(path, "r");
> - if (!fdi) {
> - p_err("can't open fdinfo: %s", strerror(errno));
> + if (!fdi)
> return NULL;
> - }
>
> while ((n = getline(&line, &line_n, fdi)) > 0) {
> char *value;
> @@ -313,7 +311,6 @@ char *get_fdinfo(int fd, const char *key)
>
> value = strchr(line, '\t');
> if (!value || !value[1]) {
> - p_err("malformed fdinfo!?");
> free(line);
> return NULL;
> }
> @@ -326,7 +323,6 @@ char *get_fdinfo(int fd, const char *key)
> return line;
> }
>
> - p_err("key '%s' not found in fdinfo", key);
> free(line);
> fclose(fdi);
> return NULL;
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index 29a3468c6cf6..1ef1ee2280a2 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -513,10 +513,9 @@ static int show_map_close_json(int fd, struct
> bpf_map_info *info)
> jsonw_uint_field(json_wtr, "owner_prog_type",
> prog_type);
> }
> - if (atoi(owner_jited))
> - jsonw_bool_field(json_wtr, "owner_jited", true);
> - else
> - jsonw_bool_field(json_wtr, "owner_jited", false);
> + if (owner_jited)
> + jsonw_bool_field(json_wtr, "owner_jited",
> + !!atoi(owner_jited));
>
> free(owner_prog_type);
> free(owner_jited);
> @@ -569,7 +568,8 @@ static int show_map_close_plain(int fd, struct
> bpf_map_info *info)
> char *owner_prog_type = get_fdinfo(fd, "owner_prog_type");
> char *owner_jited = get_fdinfo(fd, "owner_jited");
>
> - printf("\n\t");
> + if (owner_prog_type || owner_jited)
> + printf("\n\t");
> if (owner_prog_type) {
> unsigned int prog_type = atoi(owner_prog_type);
>
> @@ -579,10 +579,9 @@ static int show_map_close_plain(int fd, struct
> bpf_map_info *info)
> else
> printf("owner_prog_type %d ", prog_type);
> }
> - if (atoi(owner_jited))
> - printf("owner jited");
> - else
> - printf("owner not jited");
> + if (owner_jited)
> + printf("owner%s jited",
> + atoi(owner_jited) ? "" : " not");
>
> free(owner_prog_type);
> free(owner_jited);
> --
> 2.19.2
>
