diff mbox series

[bpf-next,v3,1/3] bpf, x86: allow function arguments up to 12 for TRACING

Message ID 20230607125911.145345-2-imagedong@tencent.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpf, x86: allow function arguments up to 12 for TRACING | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-6 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-11 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-15 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs on s390x with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 7 maintainers not CCed: tglx@linutronix.de hpa@zytor.com mingo@redhat.com dave.hansen@linux.intel.com jolsa@kernel.org haoluo@google.com bp@alien8.de
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 10 this patch: 10
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Menglong Dong June 7, 2023, 12:59 p.m. UTC
From: Menglong Dong <imagedong@tencent.com>

For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
on the kernel functions whose arguments count less than 6. This is not
friendly at all, as too many functions have arguments count more than 6.

Therefore, let's enhance it by increasing the function arguments count
allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.

For the case that we don't need to call origin function, which means
without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
that stored in the frame of the caller to current frame. The arguments
of arg6-argN are stored in "$rbp + 0x18", we need copy them to
"$rbp - regs_off + (6 * 8)".

For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
in stack before call origin function, which means we need alloc extra
"8 * (arg_count - 6)" memory in the top of the stack. Note, there should
not be any data be pushed to the stack before call the origin function.
Then, we have to store rbx with 'mov' instead of 'push'.

We use EMIT3_off32() or EMIT4() for "lea" and "sub". The range of the
imm in "lea" and "sub" is [-128, 127] if EMIT4() is used. Therefore,
we use EMIT3_off32() instead if the imm out of the range.

It works well for the FENTRY and FEXIT, I'm not sure if there are other
complicated cases.

Reviewed-by: Jiang Biao <benbjiang@tencent.com>
Signed-off-by: Menglong Dong <imagedong@tencent.com>
---
v3:
- use EMIT3_off32() for "lea" and "sub" only on necessary
- make 12 as the maximum arguments count
v2:
- instead EMIT4 with EMIT3_off32 for "lea" to prevent overflow
- make MAX_BPF_FUNC_ARGS as the maximum argument count
---
 arch/x86/net/bpf_jit_comp.c | 110 +++++++++++++++++++++++++++++++-----
 1 file changed, 96 insertions(+), 14 deletions(-)

Comments

Alexei Starovoitov June 7, 2023, 8:09 p.m. UTC | #1
On Wed, Jun 07, 2023 at 08:59:09PM +0800, menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
> on the kernel functions whose arguments count less than 6. This is not
> friendly at all, as too many functions have arguments count more than 6.
> 
> Therefore, let's enhance it by increasing the function arguments count
> allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
> 
> For the case that we don't need to call origin function, which means
> without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
> that stored in the frame of the caller to current frame. The arguments
> of arg6-argN are stored in "$rbp + 0x18", we need copy them to
> "$rbp - regs_off + (6 * 8)".
> 
> For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
> in stack before call origin function, which means we need alloc extra
> "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
> not be any data be pushed to the stack before call the origin function.
> Then, we have to store rbx with 'mov' instead of 'push'.

x86-64 psABI requires stack to be 16-byte aligned when args are passed on the stack.
I don't see this logic in the patch.
Menglong Dong June 8, 2023, 3:17 a.m. UTC | #2
On Thu, Jun 8, 2023 at 4:09 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Jun 07, 2023 at 08:59:09PM +0800, menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <imagedong@tencent.com>
> >
> > For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
> > on the kernel functions whose arguments count less than 6. This is not
> > friendly at all, as too many functions have arguments count more than 6.
> >
> > Therefore, let's enhance it by increasing the function arguments count
> > allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
> >
> > For the case that we don't need to call origin function, which means
> > without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
> > that stored in the frame of the caller to current frame. The arguments
> > of arg6-argN are stored in "$rbp + 0x18", we need copy them to
> > "$rbp - regs_off + (6 * 8)".
> >
> > For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
> > in stack before call origin function, which means we need alloc extra
> > "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
> > not be any data be pushed to the stack before call the origin function.
> > Then, we have to store rbx with 'mov' instead of 'push'.
>
> x86-64 psABI requires stack to be 16-byte aligned when args are passed on the stack.
> I don't see this logic in the patch.

Yeah, it seems I missed this logic......:)

I have not figure out the rule of the alignment, but after
observing the behavior of the compiler, the stack seems
should be like this:

------ stack frame begin
rbp

xxx   -- this part should be aligned in 16-byte

------ end of arguments in stack
xxx
------ begin of arguments in stack

So the code should be:

+       if (nr_regs > 6 && (flags & BPF_TRAMP_F_CALL_ORIG)) {
+                stack_size = ALIGN(stack_size, 16);
+                stack_size += (nr_regs - 6) * 8;
+       }

Am I right?

Thanks!
Menglong Dong
Yonghong Song June 8, 2023, 9:07 p.m. UTC | #3
On 6/7/23 5:59 AM, menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
> on the kernel functions whose arguments count less than 6. This is not
> friendly at all, as too many functions have arguments count more than 6.

Since you already have some statistics, maybe listed in the commit message.

> 
> Therefore, let's enhance it by increasing the function arguments count
> allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
> 
> For the case that we don't need to call origin function, which means
> without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
> that stored in the frame of the caller to current frame. The arguments
> of arg6-argN are stored in "$rbp + 0x18", we need copy them to
> "$rbp - regs_off + (6 * 8)".

Maybe I missed something, could you explain why it is '$rbp + 0x18'?

In the current upstream code, we have

         /* Generated trampoline stack layout:
          *
          * RBP + 8         [ return address  ]
          * RBP + 0         [ RBP             ]
          *
          * RBP - 8         [ return value    ]  BPF_TRAMP_F_CALL_ORIG or
          * 
BPF_TRAMP_F_RET_FENTRY_RET flags
          *
          *                 [ reg_argN        ]  always
          *                 [ ...             ]
          * RBP - regs_off  [ reg_arg1        ]  program's ctx pointer
          *
          * RBP - nregs_off [ regs count      ]  always
          *
          * RBP - ip_off    [ traced function ]  BPF_TRAMP_F_IP_ARG flag
          *
          * RBP - run_ctx_off [ bpf_tramp_run_ctx ]
          */

Next on-stack argument will be RBP + 16, right?

> 
> For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
> in stack before call origin function, which means we need alloc extra
> "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
> not be any data be pushed to the stack before call the origin function.
> Then, we have to store rbx with 'mov' instead of 'push'.
> 
> We use EMIT3_off32() or EMIT4() for "lea" and "sub". The range of the
> imm in "lea" and "sub" is [-128, 127] if EMIT4() is used. Therefore,
> we use EMIT3_off32() instead if the imm out of the range.
> 
> It works well for the FENTRY and FEXIT, I'm not sure if there are other
> complicated cases.

MODIFY_RETURN is also impacted by this patch.

> 
> Reviewed-by: Jiang Biao <benbjiang@tencent.com>
> Signed-off-by: Menglong Dong <imagedong@tencent.com>
[...]
Yonghong Song June 8, 2023, 9:12 p.m. UTC | #4
On 6/7/23 8:17 PM, Menglong Dong wrote:
> On Thu, Jun 8, 2023 at 4:09 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Wed, Jun 07, 2023 at 08:59:09PM +0800, menglong8.dong@gmail.com wrote:
>>> From: Menglong Dong <imagedong@tencent.com>
>>>
>>> For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
>>> on the kernel functions whose arguments count less than 6. This is not
>>> friendly at all, as too many functions have arguments count more than 6.
>>>
>>> Therefore, let's enhance it by increasing the function arguments count
>>> allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
>>>
>>> For the case that we don't need to call origin function, which means
>>> without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
>>> that stored in the frame of the caller to current frame. The arguments
>>> of arg6-argN are stored in "$rbp + 0x18", we need copy them to
>>> "$rbp - regs_off + (6 * 8)".
>>>
>>> For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
>>> in stack before call origin function, which means we need alloc extra
>>> "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
>>> not be any data be pushed to the stack before call the origin function.
>>> Then, we have to store rbx with 'mov' instead of 'push'.
>>
>> x86-64 psABI requires stack to be 16-byte aligned when args are passed on the stack.
>> I don't see this logic in the patch.
> 
> Yeah, it seems I missed this logic......:)
> 
> I have not figure out the rule of the alignment, but after
> observing the behavior of the compiler, the stack seems
> should be like this:
> 
> ------ stack frame begin
> rbp
> 
> xxx   -- this part should be aligned in 16-byte
> 
> ------ end of arguments in stack
> xxx
> ------ begin of arguments in stack
> 
> So the code should be:
> 
> +       if (nr_regs > 6 && (flags & BPF_TRAMP_F_CALL_ORIG)) {
> +                stack_size = ALIGN(stack_size, 16);
> +                stack_size += (nr_regs - 6) * 8;
> +       }
> 
> Am I right?

This is the stack_size, you should ensure stack pointer is 16-byte aligned.

> 
> Thanks!
> Menglong Dong
Menglong Dong June 9, 2023, 2:12 a.m. UTC | #5
On Fri, Jun 9, 2023 at 5:07 AM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 6/7/23 5:59 AM, menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <imagedong@tencent.com>
> >
> > For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
> > on the kernel functions whose arguments count less than 6. This is not
> > friendly at all, as too many functions have arguments count more than 6.
>
> Since you already have some statistics, maybe listed in the commit message.
>
> >
> > Therefore, let's enhance it by increasing the function arguments count
> > allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
> >
> > For the case that we don't need to call origin function, which means
> > without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
> > that stored in the frame of the caller to current frame. The arguments
> > of arg6-argN are stored in "$rbp + 0x18", we need copy them to
> > "$rbp - regs_off + (6 * 8)".
>
> Maybe I missed something, could you explain why it is '$rbp + 0x18'?
>
> In the current upstream code, we have
>
>          /* Generated trampoline stack layout:
>           *
>           * RBP + 8         [ return address  ]
>           * RBP + 0         [ RBP             ]
>           *
>           * RBP - 8         [ return value    ]  BPF_TRAMP_F_CALL_ORIG or
>           *
> BPF_TRAMP_F_RET_FENTRY_RET flags
>           *
>           *                 [ reg_argN        ]  always
>           *                 [ ...             ]
>           * RBP - regs_off  [ reg_arg1        ]  program's ctx pointer
>           *
>           * RBP - nregs_off [ regs count      ]  always
>           *
>           * RBP - ip_off    [ traced function ]  BPF_TRAMP_F_IP_ARG flag
>           *
>           * RBP - run_ctx_off [ bpf_tramp_run_ctx ]
>           */
>
> Next on-stack argument will be RBP + 16, right?
>

Sorry for the confusing, it seems there should be
some comments here.

It's not the next on-stack argument, but the next next on-stack
argument. The call chain is:

caller -> origin call -> trampoline

So, we have to skip the "RIP" in the stack frame of "origin call",
which means RBP + 16 + 8. To be clear, there are only 8-byte
in the stack frame of "origin call".

Thanks!
Menglong Dong


> >
> > For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
> > in stack before call origin function, which means we need alloc extra
> > "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
> > not be any data be pushed to the stack before call the origin function.
> > Then, we have to store rbx with 'mov' instead of 'push'.
> >
> > We use EMIT3_off32() or EMIT4() for "lea" and "sub". The range of the
> > imm in "lea" and "sub" is [-128, 127] if EMIT4() is used. Therefore,
> > we use EMIT3_off32() instead if the imm out of the range.
> >
> > It works well for the FENTRY and FEXIT, I'm not sure if there are other
> > complicated cases.
>
> MODIFY_RETURN is also impacted by this patch.
>
> >
> > Reviewed-by: Jiang Biao <benbjiang@tencent.com>
> > Signed-off-by: Menglong Dong <imagedong@tencent.com>
> [...]
Menglong Dong June 9, 2023, 2:34 a.m. UTC | #6
On Fri, Jun 9, 2023 at 5:12 AM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 6/7/23 8:17 PM, Menglong Dong wrote:
> > On Thu, Jun 8, 2023 at 4:09 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> >>
> >> On Wed, Jun 07, 2023 at 08:59:09PM +0800, menglong8.dong@gmail.com wrote:
> >>> From: Menglong Dong <imagedong@tencent.com>
> >>>
> >>> For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
> >>> on the kernel functions whose arguments count less than 6. This is not
> >>> friendly at all, as too many functions have arguments count more than 6.
> >>>
> >>> Therefore, let's enhance it by increasing the function arguments count
> >>> allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
> >>>
> >>> For the case that we don't need to call origin function, which means
> >>> without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
> >>> that stored in the frame of the caller to current frame. The arguments
> >>> of arg6-argN are stored in "$rbp + 0x18", we need copy them to
> >>> "$rbp - regs_off + (6 * 8)".
> >>>
> >>> For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
> >>> in stack before call origin function, which means we need alloc extra
> >>> "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
> >>> not be any data be pushed to the stack before call the origin function.
> >>> Then, we have to store rbx with 'mov' instead of 'push'.
> >>
> >> x86-64 psABI requires stack to be 16-byte aligned when args are passed on the stack.
> >> I don't see this logic in the patch.
> >
> > Yeah, it seems I missed this logic......:)
> >
> > I have not figure out the rule of the alignment, but after
> > observing the behavior of the compiler, the stack seems
> > should be like this:
> >
> > ------ stack frame begin
> > rbp
> >
> > xxx   -- this part should be aligned in 16-byte
> >
> > ------ end of arguments in stack
> > xxx
> > ------ begin of arguments in stack
> >
> > So the code should be:
> >
> > +       if (nr_regs > 6 && (flags & BPF_TRAMP_F_CALL_ORIG)) {
> > +                stack_size = ALIGN(stack_size, 16);
> > +                stack_size += (nr_regs - 6) * 8;
> > +       }
> >
> > Am I right?
>
> This is the stack_size, you should ensure stack pointer is 16-byte aligned.

Oh, I see. Considering the begin of the stack frame
should already be 16-byte aligned, what we should
do here is to make the size of the current stack frame
16-byte aligned. Then, rsp will be 16-byte aligned.

Am I right?

Which means the code should be:

+       if (nr_regs > 6 && (flags & BPF_TRAMP_F_CALL_ORIG)) {
+               stack_size += (nr_regs - 6) * 8;
+               stack_size = ALIGN(stack_size, 16);
+       }

Then, the size of current stack frame will be:
stack_size + 8(rbp) + 8(rip)

This is the example that I refer to:
https://godbolt.org/z/7o9nh4nbc

>
> >
> > Thanks!
> > Menglong Dong
Yonghong Song June 9, 2023, 4:29 a.m. UTC | #7
On 6/8/23 7:12 PM, Menglong Dong wrote:
> On Fri, Jun 9, 2023 at 5:07 AM Yonghong Song <yhs@meta.com> wrote:
>>
>>
>>
>> On 6/7/23 5:59 AM, menglong8.dong@gmail.com wrote:
>>> From: Menglong Dong <imagedong@tencent.com>
>>>
>>> For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
>>> on the kernel functions whose arguments count less than 6. This is not
>>> friendly at all, as too many functions have arguments count more than 6.
>>
>> Since you already have some statistics, maybe listed in the commit message.
>>
>>>
>>> Therefore, let's enhance it by increasing the function arguments count
>>> allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
>>>
>>> For the case that we don't need to call origin function, which means
>>> without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
>>> that stored in the frame of the caller to current frame. The arguments
>>> of arg6-argN are stored in "$rbp + 0x18", we need copy them to
>>> "$rbp - regs_off + (6 * 8)".
>>
>> Maybe I missed something, could you explain why it is '$rbp + 0x18'?
>>
>> In the current upstream code, we have
>>
>>           /* Generated trampoline stack layout:
>>            *
>>            * RBP + 8         [ return address  ]
>>            * RBP + 0         [ RBP             ]
>>            *
>>            * RBP - 8         [ return value    ]  BPF_TRAMP_F_CALL_ORIG or
>>            *
>> BPF_TRAMP_F_RET_FENTRY_RET flags
>>            *
>>            *                 [ reg_argN        ]  always
>>            *                 [ ...             ]
>>            * RBP - regs_off  [ reg_arg1        ]  program's ctx pointer
>>            *
>>            * RBP - nregs_off [ regs count      ]  always
>>            *
>>            * RBP - ip_off    [ traced function ]  BPF_TRAMP_F_IP_ARG flag
>>            *
>>            * RBP - run_ctx_off [ bpf_tramp_run_ctx ]
>>            */
>>
>> Next on-stack argument will be RBP + 16, right?
>>
> 
> Sorry for the confusing, it seems there should be
> some comments here.
> 
> It's not the next on-stack argument, but the next next on-stack
> argument. The call chain is:
> 
> caller -> origin call -> trampoline
> 
> So, we have to skip the "RIP" in the stack frame of "origin call",
> which means RBP + 16 + 8. To be clear, there are only 8-byte
> in the stack frame of "origin call".

Thanks. It does make sense now. So we have
   caller -> origin call -> (5 nops changed to a call) -> trampoline
          8 bytes                                    8 bytes
and inside trampoline we have 8 bytes in stack with 'push rbp'.
Yes, it would be great there is an explanation in the code.

> 
> Thanks!
> Menglong Dong
> 
> 
>>>
>>> For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
>>> in stack before call origin function, which means we need alloc extra
>>> "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
>>> not be any data be pushed to the stack before call the origin function.
>>> Then, we have to store rbx with 'mov' instead of 'push'.
>>>
>>> We use EMIT3_off32() or EMIT4() for "lea" and "sub". The range of the
>>> imm in "lea" and "sub" is [-128, 127] if EMIT4() is used. Therefore,
>>> we use EMIT3_off32() instead if the imm out of the range.
>>>
>>> It works well for the FENTRY and FEXIT, I'm not sure if there are other
>>> complicated cases.
>>
>> MODIFY_RETURN is also impacted by this patch.
>>
>>>
>>> Reviewed-by: Jiang Biao <benbjiang@tencent.com>
>>> Signed-off-by: Menglong Dong <imagedong@tencent.com>
>> [...]
diff mbox series

Patch

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 1056bbf55b17..413b986b5afd 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1868,7 +1868,7 @@  static void save_regs(const struct btf_func_model *m, u8 **prog, int nr_regs,
 	 * mov QWORD PTR [rbp-0x10],rdi
 	 * mov QWORD PTR [rbp-0x8],rsi
 	 */
-	for (i = 0, j = 0; i < min(nr_regs, 6); i++) {
+	for (i = 0, j = 0; i < min(nr_regs, MAX_BPF_FUNC_ARGS); i++) {
 		/* The arg_size is at most 16 bytes, enforced by the verifier. */
 		arg_size = m->arg_size[j];
 		if (arg_size > 8) {
@@ -1876,10 +1876,24 @@  static void save_regs(const struct btf_func_model *m, u8 **prog, int nr_regs,
 			next_same_struct = !next_same_struct;
 		}
 
-		emit_stx(prog, bytes_to_bpf_size(arg_size),
-			 BPF_REG_FP,
-			 i == 5 ? X86_REG_R9 : BPF_REG_1 + i,
-			 -(stack_size - i * 8));
+		if (i <= 5) {
+			/* copy function arguments from regs into stack */
+			emit_stx(prog, bytes_to_bpf_size(arg_size),
+				 BPF_REG_FP,
+				 i == 5 ? X86_REG_R9 : BPF_REG_1 + i,
+				 -(stack_size - i * 8));
+		} else {
+			/* copy function arguments from origin stack frame
+			 * into current stack frame.
+			 */
+			emit_ldx(prog, bytes_to_bpf_size(arg_size),
+				 BPF_REG_0, BPF_REG_FP,
+				 (i - 6) * 8 + 0x18);
+			emit_stx(prog, bytes_to_bpf_size(arg_size),
+				 BPF_REG_FP,
+				 BPF_REG_0,
+				 -(stack_size - i * 8));
+		}
 
 		j = next_same_struct ? j : j + 1;
 	}
@@ -1913,6 +1927,41 @@  static void restore_regs(const struct btf_func_model *m, u8 **prog, int nr_regs,
 	}
 }
 
+static void prepare_origin_stack(const struct btf_func_model *m, u8 **prog,
+				 int nr_regs, int stack_size)
+{
+	int i, j, arg_size;
+	bool next_same_struct = false;
+
+	if (nr_regs <= 6)
+		return;
+
+	/* Prepare the function arguments in stack before call origin
+	 * function. These arguments must be stored in the top of the
+	 * stack.
+	 */
+	for (i = 0, j = 0; i < min(nr_regs, MAX_BPF_FUNC_ARGS); i++) {
+		/* The arg_size is at most 16 bytes, enforced by the verifier. */
+		arg_size = m->arg_size[j];
+		if (arg_size > 8) {
+			arg_size = 8;
+			next_same_struct = !next_same_struct;
+		}
+
+		if (i > 5) {
+			emit_ldx(prog, bytes_to_bpf_size(arg_size),
+				 BPF_REG_0, BPF_REG_FP,
+				 (i - 6) * 8 + 0x18);
+			emit_stx(prog, bytes_to_bpf_size(arg_size),
+				 BPF_REG_FP,
+				 BPF_REG_0,
+				 -(stack_size - (i - 6) * 8));
+		}
+
+		j = next_same_struct ? j : j + 1;
+	}
+}
+
 static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
 			   struct bpf_tramp_link *l, int stack_size,
 			   int run_ctx_off, bool save_ret)
@@ -1938,7 +1987,10 @@  static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
 	/* arg1: mov rdi, progs[i] */
 	emit_mov_imm64(&prog, BPF_REG_1, (long) p >> 32, (u32) (long) p);
 	/* arg2: lea rsi, [rbp - ctx_cookie_off] */
-	EMIT4(0x48, 0x8D, 0x75, -run_ctx_off);
+	if (run_ctx_off > 0x80)
+		EMIT3_off32(0x48, 0x8D, 0xB5, -run_ctx_off);
+	else
+		EMIT4(0x48, 0x8D, 0x75, -run_ctx_off);
 
 	if (emit_rsb_call(&prog, bpf_trampoline_enter(p), prog))
 		return -EINVAL;
@@ -1954,7 +2006,10 @@  static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
 	emit_nops(&prog, 2);
 
 	/* arg1: lea rdi, [rbp - stack_size] */
-	EMIT4(0x48, 0x8D, 0x7D, -stack_size);
+	if (stack_size > 0x80)
+		EMIT3_off32(0x48, 0x8D, 0xBD, -stack_size);
+	else
+		EMIT4(0x48, 0x8D, 0x7D, -stack_size);
 	/* arg2: progs[i]->insnsi for interpreter */
 	if (!p->jited)
 		emit_mov_imm64(&prog, BPF_REG_2,
@@ -1984,7 +2039,10 @@  static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
 	/* arg2: mov rsi, rbx <- start time in nsec */
 	emit_mov_reg(&prog, true, BPF_REG_2, BPF_REG_6);
 	/* arg3: lea rdx, [rbp - run_ctx_off] */
-	EMIT4(0x48, 0x8D, 0x55, -run_ctx_off);
+	if (run_ctx_off > 0x80)
+		EMIT3_off32(0x48, 0x8D, 0x95, -run_ctx_off);
+	else
+		EMIT4(0x48, 0x8D, 0x55, -run_ctx_off);
 	if (emit_rsb_call(&prog, bpf_trampoline_exit(p), prog))
 		return -EINVAL;
 
@@ -2136,7 +2194,7 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 				void *func_addr)
 {
 	int i, ret, nr_regs = m->nr_args, stack_size = 0;
-	int regs_off, nregs_off, ip_off, run_ctx_off;
+	int regs_off, nregs_off, ip_off, run_ctx_off, arg_stack_off, rbx_off;
 	struct bpf_tramp_links *fentry = &tlinks[BPF_TRAMP_FENTRY];
 	struct bpf_tramp_links *fexit = &tlinks[BPF_TRAMP_FEXIT];
 	struct bpf_tramp_links *fmod_ret = &tlinks[BPF_TRAMP_MODIFY_RETURN];
@@ -2150,8 +2208,10 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 		if (m->arg_flags[i] & BTF_FMODEL_STRUCT_ARG)
 			nr_regs += (m->arg_size[i] + 7) / 8 - 1;
 
-	/* x86-64 supports up to 6 arguments. 7+ can be added in the future */
-	if (nr_regs > 6)
+	/* x86-64 supports up to MAX_BPF_FUNC_ARGS arguments. 1-6
+	 * are passed through regs, the remains are through stack.
+	 */
+	if (nr_regs > MAX_BPF_FUNC_ARGS)
 		return -ENOTSUPP;
 
 	/* Generated trampoline stack layout:
@@ -2170,7 +2230,14 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 	 *
 	 * RBP - ip_off    [ traced function ]  BPF_TRAMP_F_IP_ARG flag
 	 *
+	 * RBP - rbx_off   [ rbx value       ]  always
+	 *
 	 * RBP - run_ctx_off [ bpf_tramp_run_ctx ]
+	 *
+	 *                     [ stack_argN ]  BPF_TRAMP_F_CALL_ORIG
+	 *                     [ ...        ]
+	 *                     [ stack_arg2 ]
+	 * RBP - arg_stack_off [ stack_arg1 ]
 	 */
 
 	/* room for return value of orig_call or fentry prog */
@@ -2190,9 +2257,17 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 
 	ip_off = stack_size;
 
+	stack_size += 8;
+	rbx_off = stack_size;
+
 	stack_size += (sizeof(struct bpf_tramp_run_ctx) + 7) & ~0x7;
 	run_ctx_off = stack_size;
 
+	if (nr_regs > 6 && (flags & BPF_TRAMP_F_CALL_ORIG))
+		stack_size += (nr_regs - 6) * 8;
+
+	arg_stack_off = stack_size;
+
 	if (flags & BPF_TRAMP_F_SKIP_FRAME) {
 		/* skip patched call instruction and point orig_call to actual
 		 * body of the kernel function.
@@ -2212,8 +2287,14 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 	x86_call_depth_emit_accounting(&prog, NULL);
 	EMIT1(0x55);		 /* push rbp */
 	EMIT3(0x48, 0x89, 0xE5); /* mov rbp, rsp */
-	EMIT4(0x48, 0x83, 0xEC, stack_size); /* sub rsp, stack_size */
-	EMIT1(0x53);		 /* push rbx */
+	if (stack_size > 0x7F)
+		/* sub rsp, stack_size */
+		EMIT3_off32(0x48, 0x81, 0xEC, stack_size);
+	else
+		/* sub rsp, stack_size */
+		EMIT4(0x48, 0x83, 0xEC, stack_size);
+	/* mov QWORD PTR [rbp - rbx_off], rbx */
+	emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_6, -rbx_off);
 
 	/* Store number of argument registers of the traced function:
 	 *   mov rax, nr_regs
@@ -2262,6 +2343,7 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 
 	if (flags & BPF_TRAMP_F_CALL_ORIG) {
 		restore_regs(m, &prog, nr_regs, regs_off);
+		prepare_origin_stack(m, &prog, nr_regs, arg_stack_off);
 
 		if (flags & BPF_TRAMP_F_ORIG_STACK) {
 			emit_ldx(&prog, BPF_DW, BPF_REG_0, BPF_REG_FP, 8);
@@ -2321,7 +2403,7 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 	if (save_ret)
 		emit_ldx(&prog, BPF_DW, BPF_REG_0, BPF_REG_FP, -8);
 
-	EMIT1(0x5B); /* pop rbx */
+	emit_ldx(&prog, BPF_DW, BPF_REG_6, BPF_REG_FP, -rbx_off);
 	EMIT1(0xC9); /* leave */
 	if (flags & BPF_TRAMP_F_SKIP_FRAME)
 		/* skip our return address and return to parent */