netdev - Re: [PATCH bpf-next v2] tools/bpf: add log_level to bpf_load_program

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: <20190207065330.3krsb3ll5wi4kiz3@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 6 Feb 2019 22:53:32 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     netdev@...r.kernel.org, Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>, kernel-team@...com
Subject: Re: [PATCH bpf-next v2] tools/bpf: add log_level to
 bpf_load_program_attr

On Wed, Feb 06, 2019 at 10:15:50PM -0800, Yonghong Song wrote:
> The kernel verifier has three levels of logs:
>     0: no logs
>     1: logs mostly useful
>   > 1: verbose
> 
> Current libbpf API functions bpf_load_program_xattr() and
> bpf_load_program() cannot specify log_level.
> The bcc, however, provides an interface for user to
> specify log_level 2 for verbose output.
> 
> This patch added log_level into structure
> bpf_load_program_attr, so users, including bcc, can use
> bpf_load_program_xattr() to change log_level. The
> supported log_level is 0, 1, and 2.
> 
> The bpf selftest test_sock.c is modified to enable log_level = 2.
> If the "verbose" in test_sock.c is changed to true,
> the test will output logs like below:
>   $ ./test_sock
>   func#0 @0
>   0: R1=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
>   0: (bf) r6 = r1
>   1: R1=ctx(id=0,off=0,imm=0) R6_w=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
>   1: (61) r7 = *(u32 *)(r6 +28)
>   invalid bpf_context access off=28 size=4
> 
>   Test case: bind4 load with invalid access: src_ip6 .. [PASS]
>   ...
>   Test case: bind6 allow all .. [PASS]
>   Summary: 16 PASSED, 0 FAILED
> 
> Some test_sock tests are negative tests and verbose verifier
> log will be printed out as shown in the above.
> 
> Signed-off-by: Yonghong Song <yhs@...com>
> ---
>  tools/lib/bpf/bpf.c                     | 7 ++++++-
>  tools/lib/bpf/bpf.h                     | 1 +
>  tools/testing/selftests/bpf/test_sock.c | 9 ++++++++-
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> Changelog:
>   v1 -> v2:
>     . make log_level as the last member of struct bpf_load_program_attr.
>     . return -EINVAL if bpf_load_program_attr.log_level > 2.
> 
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 3defad77dc7a..6734c167279d 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -214,12 +214,17 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,
>  {
>  	void *finfo = NULL, *linfo = NULL;
>  	union bpf_attr attr;
> +	__u32 log_level;
>  	__u32 name_len;
>  	int fd;
>  
>  	if (!load_attr)
>  		return -EINVAL;
>  
> +	log_level = load_attr->log_level;
> +	if (log_level > 2)
> +		return -EINVAL;
> +
>  	name_len = load_attr->name ? strlen(load_attr->name) : 0;
>  
>  	bzero(&attr, sizeof(attr));
> @@ -292,7 +297,7 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,
>  	/* Try again with log */
>  	attr.log_buf = ptr_to_u64(log_buf);
>  	attr.log_size = log_buf_sz;
> -	attr.log_level = 1;
> +	attr.log_level = (log_level == 2) ? log_level : 1;

I think that if user specified non zero log_level in xattr
they probably want to get the log even when program was successfully loaded?
Whereas above hunk will make an effect only when program is rejected.
In addition non-zero log_level and !log_buf should be immediate error
without calling kernel?