On 9/9/24 20:08, Xu Kuohai wrote:
> On 9/9/2024 6:38 PM, Leon Hwang wrote:
>>
>>
>> On 9/9/24 17:02, Xu Kuohai wrote:
>>> On 9/8/2024 9:01 PM, Leon Hwang wrote:
>>>>
>>>>
>>>> On 1/9/24 21:38, Leon Hwang wrote:
>>>>> Like "bpf, x64: Fix tailcall infinite loop caused by freplace", the
>>>>> same
>>>>> issue happens on arm64, too.
>>>>>
[...]
>>>>>
>>>> Hi Puranjay and Kuohai,
>>>>
>>>> As it's not recommended to introduce arch_bpf_run(), this is my
>>>> approach
>>>> to fix the niche case on arm64.
>>>>
>>>> Do you have any better idea to fix it?
>>>>
>>>
>>> IIUC, the recommended appraoch is to teach verifier to reject the
>>> freplace + tailcall combination. If this combiation is allowed, we
>>> will face more than just this issue. For example, what happens if
>>> a freplace prog is attached to tail callee? The freplace prog is not
>>> reachable through the tail call, right?
>>>
>>
>> It's to reject the freplace + tailcall combination partially, see "bpf,
>> x64: Fix tailcall infinite loop caused by freplace". (Oh, I should
>> separate the rejection to a standalone patch.)
>> It rejects the case that freplace prog has tailcall and its attach
>> target has no tailcall.
>>
>> As for your example, it depends on:
>>
>> freplace target reject?
>> Has tailcall? YES NO YES
>> Has tailcall? YES YES NO
>> Has tailcall? NO YES NO
>> Has tailcall? NO YES NO
>>
>> Then, freplace prog can be tail callee always. I haven't seen any bad
>> case when freplace prog is tail callee.
>>
>
> Here is a concrete case. prog1 tail calls prog2, and prog2_new is
> attached to prog2 via freplace.
>
> SEC("tc")
> int prog1(struct __sk_buff *skb)
> {
> bpf_tail_call_static(skb, &progs, 0); // tail call prog2
> return 0;
> }
>
> SEC("tc")
> int prog2(struct __sk_buff *skb)
> {
> return 0;
> }
>
> SEC("freplace")
> int prog2_new(struct __sk_buff *skb) // target is prog2
> {
> return 0;
> }
>
> In this case, prog2_new is not reachable, since the tail call
> target in prog2 is start address of prog2 + TAIL_CALL_OFFSET,
> which locates behind freplace/fentry callsite of prog2.
>
This is an abnormal use case. We can do nothing with it, e.g. we're
unable to notify user that prog2_new is not reachable for this case.
Thanks,
Leon
