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]
Message-ID: <3a5f1d73-f9f5-a640-6f15-d5202549d467@isovalent.com>
Date:   Fri, 24 Apr 2020 11:32:34 +0100
From:   Quentin Monnet <quentin@...valent.com>
To:     Andrii Nakryiko <andriin@...com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, ast@...com, daniel@...earbox.net
Cc:     andrii.nakryiko@...il.com, kernel-team@...com
Subject: Re: [PATCH bpf-next 08/10] bpftool: add bpf_link show and pin support

2020-04-23 22:35 UTC-0700 ~ Andrii Nakryiko <andriin@...com>
> Add `bpftool link show` and `bpftool link pin` commands.
> 
> Example plain output for `link show` (with showing pinned paths):
> 
> [vmuser@...hvm bpf]$ sudo ~/local/linux/tools/bpf/bpftool/bpftool -f link
> 1: tracing  prog 12
>         prog_type tracing  attach_type fentry
>         pinned /sys/fs/bpf/my_test_link
>         pinned /sys/fs/bpf/my_test_link2
> 2: tracing  prog 13
>         prog_type tracing  attach_type fentry
> 3: tracing  prog 14
>         prog_type tracing  attach_type fentry
> 4: tracing  prog 15
>         prog_type tracing  attach_type fentry
> 5: tracing  prog 16
>         prog_type tracing  attach_type fentry
> 6: tracing  prog 17
>         prog_type tracing  attach_type fentry
> 7: raw_tracepoint  prog 21
>         tp 'sys_enter'
> 8: cgroup  prog 25
>         cgroup_id 584  attach_type egress
> 9: cgroup  prog 25
>         cgroup_id 599  attach_type egress
> 10: cgroup  prog 25
>         cgroup_id 614  attach_type egress
> 11: cgroup  prog 25
>         cgroup_id 629  attach_type egress
> 
> Signed-off-by: Andrii Nakryiko <andriin@...com>
> ---
>  tools/bpf/bpftool/common.c |   2 +
>  tools/bpf/bpftool/link.c   | 402 +++++++++++++++++++++++++++++++++++++
>  tools/bpf/bpftool/main.c   |   6 +-
>  tools/bpf/bpftool/main.h   |   5 +
>  4 files changed, 414 insertions(+), 1 deletion(-)
>  create mode 100644 tools/bpf/bpftool/link.c
> 
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index f2223dbdfb0a..c47bdc65de8e 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -262,6 +262,8 @@ int get_fd_type(int fd)
>  		return BPF_OBJ_MAP;
>  	else if (strstr(buf, "bpf-prog"))
>  		return BPF_OBJ_PROG;
> +	else if (strstr(buf, "bpf-link"))
> +		return BPF_OBJ_LINK;
>  
>  	return BPF_OBJ_UNKNOWN;
>  }
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> new file mode 100644
> index 000000000000..d5dcf9e46536
> --- /dev/null
> +++ b/tools/bpf/bpftool/link.c

[...]

> +
> +static int link_parse_fd(int *argc, char ***argv)
> +{
> +	int *fds = NULL;
> +	int nb_fds, fd;
> +
> +	fds = malloc(sizeof(int));
> +	if (!fds) {
> +		p_err("mem alloc failed");
> +		return -1;
> +	}
> +	nb_fds = link_parse_fds(argc, argv, &fds);
> +	if (nb_fds != 1) {
> +		if (nb_fds > 1) {
> +			p_err("several links match this handle");

Can this ever happen? "bpftool prog show" has this because "name" or
"tag" can match multiple programs. But "id" and "pinned" for link should
not, as far as I understand.

> +			while (nb_fds--)
> +				close(fds[nb_fds]);
> +		}
> +		fd = -1;
> +		goto exit_free;
> +	}
> +
> +	fd = fds[0];
> +exit_free:
> +	free(fds);
> +	return fd;
> +}
> +
> +static void
> +show_link_header_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> +	jsonw_uint_field(wtr, "id", info->id);
> +	if (info->type < ARRAY_SIZE(link_type_name))
> +		jsonw_string_field(wtr, "type", link_type_name[info->type]);
> +	else
> +		jsonw_uint_field(wtr, "type", info->type);
> +
> +	jsonw_uint_field(json_wtr, "prog_id", info->prog_id);
> +}
> +
> +static int get_prog_info(int prog_id, struct bpf_prog_info *info)
> +{
> +	__u32 len = sizeof(*info);
> +	int err, prog_fd;
> +
> +	prog_fd = bpf_prog_get_fd_by_id(prog_id);
> +	if (prog_fd < 0)
> +		return prog_fd;
> +
> +	memset(info, 0, sizeof(*info));
> +	err = bpf_obj_get_info_by_fd(prog_fd, info, &len);
> +	if (err) {
> +		p_err("can't get prog info: %s", strerror(errno));
> +		close(prog_fd);
> +		return err;

Nit: you could "return err;" at the end of the function, and remove the
"close()" and "return" from this if block.

> +	}
> +
> +	close(prog_fd);
> +	return 0;
> +}
> +

[...]

> +
> +static int do_show_subset(int argc, char **argv)
> +{
> +	int *fds = NULL;
> +	int nb_fds, i;
> +	int err = -1;
> +
> +	fds = malloc(sizeof(int));
> +	if (!fds) {
> +		p_err("mem alloc failed");
> +		return -1;
> +	}
> +	nb_fds = link_parse_fds(&argc, &argv, &fds);
> +	if (nb_fds < 1)
> +		goto exit_free;
> +
> +	if (json_output && nb_fds > 1)
> +		jsonw_start_array(json_wtr);	/* root array */
> +	for (i = 0; i < nb_fds; i++) {
> +		err = do_show_link(fds[i]);
> +		if (err) {
> +			for (; i + 1 < nb_fds; i++)
> +				close(fds[i]);
> +			break;
> +		}
> +	}
> +	if (json_output && nb_fds > 1)
> +		jsonw_end_array(json_wtr);	/* root array */
> +
> +exit_free:
> +	free(fds);
> +	return err;
> +}
> +
> +static int do_show(int argc, char **argv)
> +{
> +	__u32 id = 0;
> +	int err;
> +	int fd;
> +
> +	if (show_pinned)
> +		build_pinned_obj_table(&link_table, BPF_OBJ_LINK);
> +
> +	if (argc == 2)
> +		return do_show_subset(argc, argv);

I understand the "_subset" aspect was taken from prog.c. But it was
added there (ec2025095cf6) because "bpftool prog show <name|tag>" can
match several programs, and the array with fds would be reallocated as
required while parsing names/tags.

I do not think that by restricting the selection on "id" or "pinned",
you can get more than one link at a time. So we can probably avoid
juggling with fd arrays for link_parse_fd() / link_parse_fds() and show
just the single relevant link.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ