[PATCH bpf-next] bpf: check for insn arrays in check_ptr_alignment

[PATCH bpf-next] bpf: check for insn arrays in check_ptr_alignment

[Anton Protopopov]

Do not abuse the strict_alignment_once flag, and check if the map is
an instruction array inside the check_ptr_alignment() function.

Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
---
 kernel/bpf/verifier.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 58f99557ba38..ddc68273d29f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6482,6 +6482,8 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
 		break;
 	case PTR_TO_MAP_VALUE:
 		pointer_desc = "value ";
+		if (reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY)
+			strict = true;
 		break;
 	case PTR_TO_CTX:
 		pointer_desc = "context ";
@@ -7529,8 +7531,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 {
 	struct bpf_reg_state *regs = cur_regs(env);
 	struct bpf_reg_state *reg = regs + regno;
-	bool insn_array = reg->type == PTR_TO_MAP_VALUE &&
-			  reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY;
 	int size, err = 0;
 
 	size = bpf_size_to_bytes(bpf_size);
@@ -7538,7 +7538,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 		return size;
 
 	/* alignment checks will add in reg->off themselves */
-	err = check_ptr_alignment(env, reg, off, size, strict_alignment_once || insn_array);
+	err = check_ptr_alignment(env, reg, off, size, strict_alignment_once);
 	if (err)
 		return err;
 
-- 
2.34.1

Re: [PATCH bpf-next] bpf: check for insn arrays in check_ptr_alignment

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 766695491..43991449f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6482,6 +6482,8 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
>  		break;
>  	case PTR_TO_MAP_VALUE:
>  		pointer_desc = "value ";
> +		if (reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY)
> +			strict = true;
>  		break;
>  	case PTR_TO_CTX:
>  		pointer_desc = "context ";
> @@ -7529,8 +7531,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>  {
>  	struct bpf_reg_state *regs = cur_regs(env);
>  	struct bpf_reg_state *reg = regs + regno;
> -	bool insn_array = reg->type == PTR_TO_MAP_VALUE &&
> -			  reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY;
>  	int size, err = 0;
>
>  	size = bpf_size_to_bytes(bpf_size);
> @@ -7538,7 +7538,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>  		return size;
>
>  	/* alignment checks will add in reg->off themselves */
> -	err = check_ptr_alignment(env, reg, off, size, strict_alignment_once || insn_array);
> +	err = check_ptr_alignment(env, reg, off, size, strict_alignment_once);
>  	if (err)
>  		return err;

The insn_array variable declaration was removed, but this code still
uses it at check_mem_access():7568. Does this compile?

	if (t == BPF_WRITE && insn_array) {
		verbose(env, "writes into insn_array not allowed\n");
		return -EACCES;
	}

The fix would be to check the map type directly:

	if (t == BPF_WRITE && reg->type == PTR_TO_MAP_VALUE &&
	    reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY) {


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19747897022

Re: [PATCH bpf-next] bpf: check for insn arrays in check_ptr_alignment

[Anton Protopopov]

On 25/11/27 09:25PM, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 766695491..43991449f 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -6482,6 +6482,8 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
> >  		break;
> >  	case PTR_TO_MAP_VALUE:
> >  		pointer_desc = "value ";
> > +		if (reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY)
> > +			strict = true;
> >  		break;
> >  	case PTR_TO_CTX:
> >  		pointer_desc = "context ";
> > @@ -7529,8 +7531,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> >  {
> >  	struct bpf_reg_state *regs = cur_regs(env);
> >  	struct bpf_reg_state *reg = regs + regno;
> > -	bool insn_array = reg->type == PTR_TO_MAP_VALUE &&
> > -			  reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY;
> >  	int size, err = 0;
> >
> >  	size = bpf_size_to_bytes(bpf_size);
> > @@ -7538,7 +7538,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> >  		return size;
> >
> >  	/* alignment checks will add in reg->off themselves */
> > -	err = check_ptr_alignment(env, reg, off, size, strict_alignment_once || insn_array);
> > +	err = check_ptr_alignment(env, reg, off, size, strict_alignment_once);
> >  	if (err)
> >  		return err;
> 
> The insn_array variable declaration was removed, but this code still
> uses it at check_mem_access():7568. Does this compile?

Robot is right, I should have sent it as a series with
https://lore.kernel.org/bpf/20251127210656.3239541-1-a.s.protopopov@gmail.com/
Will resend it now.

> 	if (t == BPF_WRITE && insn_array) {
> 		verbose(env, "writes into insn_array not allowed\n");
> 		return -EACCES;
> 	}
> 
> The fix would be to check the map type directly:
> 
> 	if (t == BPF_WRITE && reg->type == PTR_TO_MAP_VALUE &&
> 	    reg->map_ptr->map_type == BPF_MAP_TYPE_INSN_ARRAY) {
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19747897022