Re: [PATCH bpf-next v2 1/2] bpf: clean-up bpf_verifier_vlog() for BPF_LOG_KERNEL log level

[Daniel Borkmann <daniel@iogearbox.net>]

On 10/29/21 3:53 PM, 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, but not all btf_verifier_log()
> are ended by newline, so checking whether or not the log message
> has the trailing newline and adding a newline if not.
> 
> 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@huawei.com>
> ---
>   kernel/bpf/verifier.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 3c8aa7df1773..22f0d2292c2c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -299,13 +299,15 @@ 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);
> +		bool newline = n > 0 && log->kbuf[n - 1] == '\n';
> +
> +		pr_err("BPF:%s%s", log->kbuf, newline ? "" : "\n");

nit: Given you change this anyway, is there a reason not to go with "BPF: %s%s" instead?

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