Re: [PATCH bpf-next v4 01/16] bpf: Add verifier diagnostics report helpers

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch completely fails to implement the prose wrapping and reusable report sections promised in the commit message.
- [Low] Diagnostic category string macros are defined in the .c file instead of the header, making them inaccessible to callers.
--

--- Patch [1]: [PATCH bpf-next v4 01/16] bpf: Add verifier diagnostics report helpers ---
commit 2d39aa0d2b938aedc24539fb306c5c0aabb08c25
Author: Kumar Kartikeya Dwivedi <[email protected]>

bpf: Add verifier diagnostics report helpers

This patch adds a small diagnostics renderer for verifier reports and wires it
into the BPF build. It includes helpers to emit a failure header and aims to
provide reusable report sections.

> Wrap report prose at 100 columns so Reason and Suggestion text stays
> readable without changing source or instruction gutters.

[Severity: Medium]
Is this description accurate for the current implementation? Looking at the
code for bpf_diag_header() and diag_write() added in this patch, there does
not appear to be any text wrapping logic or implementation for reusable
report sections like Reason and Suggestion. Was this logic accidentally
omitted from this patch?

> diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
> new file mode 100644
> --- /dev/null
> +++ b/kernel/bpf/diagnostics.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (c) 2026 Meta Platforms, Inc. and affiliates.
> +
> +#include <linux/bpf_verifier.h>
> +#include <linux/ctype.h>
> +#include <linux/stdarg.h>
> +
> +#include "diagnostics.h"
> +
> +#define MEMORY_SAFETY "Memory Safety"
> +#define REGISTER_TYPE_SAFETY "Register Type Safety"
> +#define CALL_TYPE_SAFETY "Call Type Safety"
> +#define RESOURCE_LIFETIME_SAFETY "Resource Lifetime Safety"
> +#define EXECUTION_CONTEXT_SAFETY "Execution Context Safety"
> +#define PROGRAM_STRUCTURE "Program Structure"
> +#define POLICY "Policy"
> +#define VERIFIER_LIMIT "Verifier Limit"

[Severity: Low]
Should these category string macros be defined in diagnostics.h instead of
here in the source file? Since they are not used within diagnostics.c and
are not exported in the header file, other files in the verifier will not be
able to use them as standardized arguments when calling bpf_diag_header().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1