[PATCH] bpf: use 'ctx' instead of 'skb' in debug message

[PATCH] bpf: use 'ctx' instead of 'skb' in debug message

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The error message here should mention 'ctx' since the context
is now more generic than just an skb.

Signed-off-by: Johannes Berg <johannes.berg@intel.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 796b68d00119..1b3c921d3798 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2329,7 +2329,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
 		return err;
 
 	if (regs[BPF_REG_6].type != PTR_TO_CTX) {
-		verbose("at the time of BPF_LD_ABS|IND R6 != pointer to skb\n");
+		verbose("at the time of BPF_LD_ABS|IND R6 != pointer to ctx\n");
 		return -EINVAL;
 	}
 
-- 
2.11.0

Re: [PATCH] bpf: use 'ctx' instead of 'skb' in debug message

[Daniel Borkmann]

On 04/04/2017 04:46 PM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The error message here should mention 'ctx' since the context
> is now more generic than just an skb.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.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 796b68d00119..1b3c921d3798 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2329,7 +2329,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
>   		return err;
>
>   	if (regs[BPF_REG_6].type != PTR_TO_CTX) {
> -		verbose("at the time of BPF_LD_ABS|IND R6 != pointer to skb\n");
> +		verbose("at the time of BPF_LD_ABS|IND R6 != pointer to ctx\n");
>   		return -EINVAL;

Seems okay, the reason why we had 'skb' in the verbose message here is
due to BPF_LD + BPF_ABS/BPF_IND operations being only specific to skbs
and no other context (see __bpf_prog_run(), and in verifier may_access_skb()
check before that verbose() message in check_ld_abs()). Reason for this
is mostly historical due to the cBPF to eBPF migration so that these loads
don't get slowed down when migrated to eBPF and can be handled by JIT
optimizations (e.g., caching skb->data), too. Anyway, just to provide
some more background on this. I've no strong opinion if you want to change
the verifier error message, so:

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [PATCH] bpf: use 'ctx' instead of 'skb' in debug message

[Johannes Berg]

On Tue, 2017-04-04 at 19:26 +0200, Daniel Borkmann wrote:

> >   	if (regs[BPF_REG_6].type != PTR_TO_CTX) {
> > -		verbose("at the time of BPF_LD_ABS|IND R6 !=
> > pointer to skb\n");
> > +		verbose("at the time of BPF_LD_ABS|IND R6 !=
> > pointer to ctx\n");
> >   		return -EINVAL;
> 
> Seems okay, the reason why we had 'skb' in the verbose message here
> is due to BPF_LD + BPF_ABS/BPF_IND operations being only specific to
> skbs and no other context (see __bpf_prog_run(), and in verifier
> may_access_skb() check before that verbose() message in
> check_ld_abs()). Reason for this is mostly historical due to the cBPF
> to eBPF migration so that these loads don't get slowed down when
> migrated to eBPF and can be handled by JIT optimizations (e.g.,
> caching skb->data), too. Anyway, just to provide some more background
> on this. I've no strong opinion if you want to change the verifier
> error message, so:

Ah. I really have no opinion on this either - it just seemed somewhat
inconsistent. I clearly neglected to read the comment in front of the
function though, that explains that we must have ctx == skb. I think
therefore it's probably better to drop this - thanks for the
explanation!

johannes