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  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.

Powered by blists - more mailing lists