Re: [PATCH bpf-next 1/3] bpf: Support inlining bpf_kptr_xchg() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Eduard,

On 12/21/2023 1:07 AM, Eduard Zingerman wrote:
> On Tue, 2023-12-19 at 21:56 +0800, Hou Tao wrote:
>> From: Hou Tao <houtao1@xxxxxxxxxx>
> [...]
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 9456ee0ad129..7814c4f7576e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -19668,6 +19668,23 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
>>  			continue;
>>  		}
>>  
>> +		/* Implement bpf_kptr_xchg inline */
>> +		if (prog->jit_requested && BITS_PER_LONG == 64 &&
>> +		    insn->imm == BPF_FUNC_kptr_xchg &&
>> +		    bpf_jit_supports_ptr_xchg()) {
>> +			insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_2);
>> +			insn_buf[1] = BPF_ATOMIC_OP(BPF_DW, BPF_XCHG, BPF_REG_1, BPF_REG_0, 0);
>> +			cnt = 2;
>> +
>> +			new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
>> +			if (!new_prog)
>> +				return -ENOMEM;
>> +
>> +			delta    += cnt - 1;
>> +			env->prog = prog = new_prog;
>> +			insn      = new_prog->insnsi + i + delta;
>> +			continue;
>> +		}
>>  patch_call_imm:
>>  		fn = env->ops->get_func_proto(insn->imm, env->prog);
>>  		/* all functions that have prototype and verifier allowed
> Hi Hou,
>
> I have a suggestion about testing this rewrite.
> It is possible to use function get_xlated_program() from
> tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c,
> to obtain a BPF disassembly for the program after
> do_misc_fixups() are applied.
>
> So, it shouldn't be difficult to:
> - prepare a dummy program in progs/ that uses bpf_kptr_xchg();
> - prepare a new test_* function in prog_tests/ that:
>   - loads that dummy program;
>   - queries it's disassembly using get_xlated_program();
>   - compares it with expected template.
>
> I know that do_misc_fixups() are usually not tested this way,
> but that does not mean they shouldn't, wdyt?

Good idea and thanks for the detailed guidance. Will try it in v2.
>
> Thanks,
> Eduard
> .





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux