netdev - Re: [PATCH bpf-next] bpf: bpf_log() clean-up for kernel log output

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: <20211026213502.s5arrmvh42tbymvv@kafai-mbp.dhcp.thefacebook.com>
Date:   Tue, 26 Oct 2021 14:35:02 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Hou Tao <houtao1@...wei.com>
CC:     Alexei Starovoitov <ast@...nel.org>, Yonghong Song <yhs@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, <netdev@...r.kernel.org>,
        <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next] bpf: bpf_log() clean-up for kernel log output

On Tue, Oct 26, 2021 at 09:38:19PM +0800, Hou Tao wrote:
> An extra newline will output for bpf_log() with BPF_LOG_KERNEL level
> as shown below:
> 
> [   52.095704] BPF:The function test_3 has 12 arguments. Too many.
> [   52.095704]
> [   52.096896] Error in parsing func ptr test_3 in struct bpf_dummy_ops
> 
> Now all bpf_log() are ended by newline, so just remove the extra newline.
bpf_verifier_vlog is also called by btf_verifier_log.
Not all of them is ended with newline.

> 
> Also there is no need to calculate the left userspace buffer size
> for kernel log output and to truncate the output by '\0' which
> has already been done by vscnprintf(), so only do these for
> userspace log output.
> 
> Signed-off-by: Hou Tao <houtao1@...wei.com>
> ---
>  kernel/bpf/verifier.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c6616e325803..7d4a313da86e 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -299,13 +299,13 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt,
>  	WARN_ONCE(n >= BPF_VERIFIER_TMP_LOG_SIZE - 1,
>  		  "verifier log line truncated - local buffer too short\n");
>  
> -	n = min(log->len_total - log->len_used - 1, n);
> -	log->kbuf[n] = '\0';
> -
>  	if (log->level == BPF_LOG_KERNEL) {
> -		pr_err("BPF:%s\n", log->kbuf);
> +		pr_err("BPF:%s", log->kbuf);
How about trim the tailing '\n' (if any) from kbuf?
or just test if kbuf is ended with '\n'?

>  		return;
>  	}
> +
> +	n = min(log->len_total - log->len_used - 1, n);
> +	log->kbuf[n] = '\0';
>  	if (!copy_to_user(log->ubuf + log->len_used, log->kbuf, n + 1))
>  		log->len_used += n;
>  	else
> -- 
> 2.29.2
>