On 05/18/2018 08:45 PM, Daniel Borkmann wrote:
> On 05/18/2018 02:50 PM, Sandipan Das wrote:
>> The imm field of a bpf instruction is a signed 32-bit integer.
>> For JIT bpf-to-bpf function calls, it stores the offset of the
>> start address of the callee's JITed image from __bpf_call_base.
>>
>> For some architectures, such as powerpc64, this offset may be
>> as large as 64 bits and cannot be accomodated in the imm field
>> without truncation.
>>
>> We resolve this by:
>>
>> [1] Additionally using the auxillary data of each function to
>> keep a list of start addresses of the JITed images for all
>> functions determined by the verifier.
>>
>> [2] Retaining the subprog id inside the off field of the call
>> instructions and using it to index into the list mentioned
>> above and lookup the callee's address.
>>
>> To make sure that the existing JIT compilers continue to work
>> without requiring changes, we keep the imm field as it is.
>>
>> Signed-off-by: Sandipan Das <sandi...@linux.vnet.ibm.com>
>> ---
>> kernel/bpf/verifier.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index a9e4b1372da6..6c56cce9c4e3 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -5383,11 +5383,24 @@ static int jit_subprogs(struct bpf_verifier_env *env)
>> insn->src_reg != BPF_PSEUDO_CALL)
>> continue;
>> subprog = insn->off;
>> - insn->off = 0;
>> insn->imm = (u64 (*)(u64, u64, u64, u64, u64))
>> func[subprog]->bpf_func -
>> __bpf_call_base;
>> }
>> +
>> + /* we use the aux data to keep a list of the start addresses
>> + * of the JITed images for each function in the program
>> + *
>> + * for some architectures, such as powerpc64, the imm field
>> + * might not be large enough to hold the offset of the start
>> + * address of the callee's JITed image from __bpf_call_base
>> + *
>> + * in such cases, we can lookup the start address of a callee
>> + * by using its subprog id, available from the off field of
>> + * the call instruction, as an index for this list
>> + */
>> + func[i]->aux->func = func;
>> + func[i]->aux->func_cnt = env->subprog_cnt + 1;
>
> The target tree you have here is infact bpf, since in bpf-next there was a
> cleanup where the + 1 is removed. Just for the record that we need to keep
> this in mind for bpf into bpf-next merge since this would otherwise subtly
> break.
>
Sorry about the wrong tag. This series is indeed based off bpf-next.
- Sandipan
>> }
>> for (i = 0; i < env->subprog_cnt; i++) {
>> old_bpf_func = func[i]->bpf_func;
>>
>
>
