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:   Wed, 5 Dec 2018 08:50:10 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Quentin Monnet <quentin.monnet@...ronome.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace
 pipe

On Wed, Dec 05, 2018 at 10:28:24AM +0000, Quentin Monnet wrote:
> BPF programs can use the bpf_trace_printk() helper to print debug
> information into the trace pipe. Add a subcommand
> "bpftool prog tracelog" to simply dump this pipe to the console.
> 
> This is for a good part copied from iproute2, where the feature is
> available with "tc exec bpf dbg". Changes include dumping pipe content
> to stdout instead of stderr and adding JSON support (content is dumped
> as an array of strings, one per line read from the pipe). This version
> is dual-licensed, with Daniel's permission.
> 
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Suggested-by: Daniel Borkmann <daniel@...earbox.net>
> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> ---
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst |  13 +-
>  tools/bpf/bpftool/bash-completion/bpftool        |   5 +-
>  tools/bpf/bpftool/main.h                         |   1 +
>  tools/bpf/bpftool/prog.c                         |   4 +-
>  tools/bpf/bpftool/tracelog.c                     | 157 +++++++++++++++++++++++
>  5 files changed, 176 insertions(+), 4 deletions(-)
>  create mode 100644 tools/bpf/bpftool/tracelog.c
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index ab36e920e552..5524b6dccd85 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -26,8 +26,9 @@ MAP COMMANDS
>  |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
>  |	**bpftool** **prog pin** *PROG* *FILE*
>  |	**bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> -|       **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> -|       **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
> +|	**bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> +|	**bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
> +|	**bpftool** **prog tracelog**
>  |	**bpftool** **prog help**
>  |
>  |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
> @@ -117,6 +118,14 @@ DESCRIPTION
>  		  parameter, with the exception of *flow_dissector* which is
>  		  detached from the current networking name space.
>  
> +	**bpftool prog tracelog**
> +		  Dump the trace pipe of the system to the console (stdout).
> +		  Hit <Ctrl+C> to stop printing. BPF programs can write to this
> +		  trace pipe at runtime with the **bpf_trace_printk()** helper.
> +		  This should be used only for debugging purposes. For
> +		  streaming data from BPF programs to user space, one can use
> +		  perf events (see also **bpftool-map**\ (8)).
> +
>  	**bpftool prog help**
>  		  Print short help message.
>  
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 9a60080f085f..44c189ba072a 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -398,10 +398,13 @@ _bpftool()
>                              ;;
>                      esac
>                      ;;
> +                tracelog)
> +                    return 0
> +                    ;;
>                  *)
>                      [[ $prev == $object ]] && \
>                          COMPREPLY=( $( compgen -W 'dump help pin attach detach load \
> -                            show list' -- "$cur" ) )
> +                            show list tracelog' -- "$cur" ) )
>                      ;;
>              esac
>              ;;
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 2761981669c8..0be0dd8f467f 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -167,6 +167,7 @@ int do_event_pipe(int argc, char **argv);
>  int do_cgroup(int argc, char **arg);
>  int do_perf(int argc, char **arg);
>  int do_net(int argc, char **arg);
> +int do_tracelog(int argc, char **arg);
>  
>  int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
>  int prog_parse_fd(int *argc, char ***argv);
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 56db61c5a91f..54c8dbf05c9c 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1140,6 +1140,7 @@ static int do_help(int argc, char **argv)
>  		"                         [pinmaps MAP_DIR]\n"
>  		"       %s %s attach PROG ATTACH_TYPE [MAP]\n"
>  		"       %s %s detach PROG ATTACH_TYPE [MAP]\n"
> +		"       %s %s tracelog\n"
>  		"       %s %s help\n"
>  		"\n"
>  		"       " HELP_SPEC_MAP "\n"
> @@ -1158,7 +1159,7 @@ static int do_help(int argc, char **argv)
>  		"",
>  		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
>  		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> -		bin_name, argv[-2], bin_name, argv[-2]);
> +		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
>  
>  	return 0;
>  }
> @@ -1173,6 +1174,7 @@ static const struct cmd cmds[] = {
>  	{ "loadall",	do_loadall },
>  	{ "attach",	do_attach },
>  	{ "detach",	do_detach },
> +	{ "tracelog",	do_tracelog },
>  	{ 0 }
>  };
>  
> diff --git a/tools/bpf/bpftool/tracelog.c b/tools/bpf/bpftool/tracelog.c
> new file mode 100644
> index 000000000000..1fa8e513f590
> --- /dev/null
> +++ b/tools/bpf/bpftool/tracelog.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/* Copyright (c) 2015-2017 Daniel Borkmann */
> +/* Copyright (c) 2018 Netronome Systems, Inc. */
> +
> +#include <errno.h>
> +#include <limits.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <linux/magic.h>
> +#include <sys/fcntl.h>
> +#include <sys/vfs.h>
> +
> +#include "main.h"
> +
> +#ifndef TRACEFS_MAGIC
> +# define TRACEFS_MAGIC	0x74726163
> +#endif
> +
> +#define _textify(x)	#x
> +#define textify(x)	_textify(x)
> +
> +FILE *trace_pipe_fd;
> +char *buff;
> +
> +static int validate_tracefs_mnt(const char *mnt, unsigned long magic)
> +{
> +	struct statfs st_fs;
> +
> +	if (statfs(mnt, &st_fs) < 0)
> +		return -ENOENT;
> +	if ((unsigned long)st_fs.f_type != magic)
> +		return -ENOENT;
> +
> +	return 0;
> +}
> +
> +static bool
> +find_tracefs_mnt_single(unsigned long magic, char *mnt, const char *mntpt)
> +{
> +	size_t src_len;
> +
> +	if (validate_tracefs_mnt(mntpt, magic))
> +		return false;
> +
> +	src_len = strlen(mntpt);
> +	if (src_len + 1 >= PATH_MAX) {
> +		p_err("tracefs mount point name too long");
> +		return false;
> +	}
> +
> +	strcpy(mnt, mntpt);
> +	return true;
> +}
> +
> +static bool find_tracefs_pipe(char *mnt)
> +{
> +	static const char * const known_mnts[] = {
> +		"/sys/kernel/debug/tracing",
> +		"/sys/kernel/tracing",
> +		"/tracing",
> +		"/trace",

I wonder where this list came from?
I only knew of 1st one.

> +	};
> +	const char *pipe_name = "/trace_pipe";
> +	const char *fstype = "tracefs";
> +	char type[100], format[32];
> +	const char * const *ptr;
> +	bool found = false;
> +	FILE *fp;
> +
> +	for (ptr = known_mnts; ptr < known_mnts + ARRAY_SIZE(known_mnts); ptr++)
> +		if (find_tracefs_mnt_single(TRACEFS_MAGIC, mnt, *ptr))
> +			goto exit_found;
> +
> +	fp = fopen("/proc/mounts", "r");
> +	if (!fp)
> +		return false;
> +
> +	/* Allow room for NULL terminating byte and pipe file name */
> +	snprintf(format, sizeof(format), "%%*s %%%zds %%99s %%*s %%*d %%*d\\n",
> +		 PATH_MAX - strlen(pipe_name) - 1);

before scanning trace_pipe could you add a check that trace_options are compatible?
Otherwise there will be a lot of garbage printed.
afaik default is rarely changed, so the patch is ok as-is.
The followup some time in the future would be perfect.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ