netdev - Re: [PATCH bpf-next v5 4/5] bpftool: support dumping metadata

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 Sep 2020 15:03:21 -0700
From:   Stanislav Fomichev <sdf@...gle.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        YiFei Zhu <zhuyifei1999@...il.com>
Subject: Re: [PATCH bpf-next v5 4/5] bpftool: support dumping metadata

On Mon, Sep 14, 2020 at 4:39 PM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> On Mon, Sep 14, 2020 at 11:37 AM Stanislav Fomichev <sdf@...gle.com> wrote:
> > +               if (map_info.type != BPF_MAP_TYPE_ARRAY)
> > +                       continue;
> > +               if (map_info.key_size != sizeof(int))
> > +                       continue;
> > +               if (map_info.max_entries != 1)
> > +                       continue;
> > +               if (!map_info.btf_value_type_id)
> > +                       continue;
> > +               if (!strstr(map_info.name, ".rodata"))
> > +                       continue;
> > +
> > +               *map_id = map_ids[i];
>
> return value_size here to avoid extra syscall below; or rather just
> accept bpf_map_info pointer and read everything into it?
Good idea, will just return bpf_map_info.

> > +       value = malloc(map_info->value_size);
> > +       if (!value)
> > +               goto out_close;
> > +
> > +       if (bpf_map_lookup_elem(map_fd, &key, value))
> > +               goto out_free;
> > +
> > +       close(map_fd);
> > +       return value;
> > +
> > +out_free:
> > +       free(value);
> > +out_close:
> > +       close(map_fd);
> > +       return NULL;
> > +}
> > +
> > +static bool has_metadata_prefix(const char *s)
> > +{
> > +       return strstr(s, BPF_METADATA_PREFIX) == s;
>
> this is a substring check, not a prefix check, use strncmp instead
Right, but I then compare the result to the original value (== s).
So if the substring starts with 0th index, we are good.

"strncmp(s, BPF_METADATA_PREFIX, BPF_METADATA_PREFIX_LEN) == 0;" felt
a bit clunky, but I can use it anyway if it helps the readability.

> > +}
> > +
> > +static void show_prog_metadata(int fd, __u32 num_maps)
> > +{
> > +       const struct btf_type *t_datasec, *t_var;
> > +       struct bpf_map_info map_info = {};
>
> it should be memset
Sounds good.

>
> > +       } else {
> > +               json_writer_t *btf_wtr = jsonw_new(stdout);
> > +               struct btf_dumper d = {
> > +                       .btf = btf,
> > +                       .jw = btf_wtr,
> > +                       .is_plain_text = true,
> > +               };
>
> empty line here?
Sure.