Re: [PATCH bpf 3/4] bpf: Improve ctx access verifier error message

[Yonghong Song <yonghong.song@linux.dev>]

On 8/1/25 2:47 PM, Alexei Starovoitov wrote:
> On Fri, Aug 1, 2025 at 9:31 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>>
>> On 8/1/25 2:49 AM, Paul Chaignon wrote:
>>> We've already had two "error during ctx access conversion" warnings
>>> triggered by syzkaller. Let's improve the error message by dumping the
>>> cnt variable so that we can more easily differentiate between the
>>> different error cases.
>>>
>>> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
>>> ---
>>>    kernel/bpf/verifier.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 399f03e62508..0806295945e4 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -21445,7 +21445,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
>>>                                         &target_size);
>>>                if (cnt == 0 || cnt >= INSN_BUF_SIZE ||
>>>                    (ctx_field_size && !target_size)) {
>>> -                     verifier_bug(env, "error during ctx access conversion");
>>> +                     verifier_bug(env, "error during ctx access conversion (%d)", cnt);
>> For the above, users still will not know what '(%d)' mean. So if we want to
> Right, but such verifier_bug reports are mainly for developers,
> and we will know what it's about especially after redundant (1) is fixed.
>
>> provide better verification measure, we should do
>>          if (cnt == 0 || cnt >= INSN_BUF_SIZE) {
>>                  verifier_bug(env, "error during ctx access conversion (insn cnt %d)", cnt);
>>                  return -EFAULT;
>>          } else if (ctx_field_size && !target_size) {
>>                  verifier_bug(env, "error during ctx access conversion (ctx_field_size %d, target_size 0)", ctx_field_size);
>>                  return -EFAULT;
>>          }
> It's nicer, but overkill. As Paul explained if cnt > 0 && < INSN_BUF_SIZE
> it must be ctx_field_size/tager_size issue that
> needs debugging anyway with a proper reproducer.
> So making this particular debug output prettier won't help
> analysis much.

Sure. I am ok with this. Thanks!

[...]