bpf: specify the old and new poke_type for bpf_arch_text_poke

In the origin logic, the bpf_arch_text_poke() assume that the old and new
instructions have the same opcode. However, they can have different opcode
if we want to replace a "call" insn with a "jmp" insn.

Therefore, add the new function parameter "old_t" along with the "new_t",
which are used to indicate the old and new poke type. Meanwhile, adjust
the implement of bpf_arch_text_poke() for all the archs.

"BPF_MOD_NOP" is added to make the code more readable. In
bpf_arch_text_poke(), we still check if the new and old address is NULL to
determine if nop insn should be used, which I think is more safe.

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
Link: https://lore.kernel.org/r/20251118123639.688444-6-dongml2@chinatelecom.cn
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 4cfb549f2b4395d532e22d177d0c667fc3323377..929123a5431aa1479ecded2b1bc9e90422c892ef 100644 (file)
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2934,8 +2934,9 @@ static int gen_branch_or_nop(enum aarch64_insn_branch_type type, void *ip,
  * The dummy_tramp is used to prevent another CPU from jumping to unknown
  * locations during the patching process, making the patching process easier.
  */
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
-                      void *old_addr, void *new_addr)
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                      enum bpf_text_poke_type new_t, void *old_addr,
+                      void *new_addr)
 {
        int ret;
        u32 old_insn;
@@ -2979,14 +2980,13 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
                    !poking_bpf_entry))
                return -EINVAL;
 
-       if (poke_type == BPF_MOD_CALL)
-               branch_type = AARCH64_INSN_BRANCH_LINK;
-       else
-               branch_type = AARCH64_INSN_BRANCH_NOLINK;
-
+       branch_type = old_t == BPF_MOD_CALL ? AARCH64_INSN_BRANCH_LINK :
+                                             AARCH64_INSN_BRANCH_NOLINK;
        if (gen_branch_or_nop(branch_type, ip, old_addr, plt, &old_insn) < 0)
                return -EFAULT;
 
+       branch_type = new_t == BPF_MOD_CALL ? AARCH64_INSN_BRANCH_LINK :
+                                             AARCH64_INSN_BRANCH_NOLINK;
        if (gen_branch_or_nop(branch_type, ip, new_addr, plt, &new_insn) < 0)
                return -EFAULT;
 

diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index cbe53d0b7fb063351ce774948acf41a7cb2d2d3c..2e7dacbbef5c8f208d699556cd9616109dc32c87 100644 (file)
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -1284,11 +1284,12 @@ void *bpf_arch_text_copy(void *dst, void *src, size_t len)
        return ret ? ERR_PTR(-EINVAL) : dst;
 }
 
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
-                      void *old_addr, void *new_addr)
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                      enum bpf_text_poke_type new_t, void *old_addr,
+                      void *new_addr)
 {
        int ret;
-       bool is_call = (poke_type == BPF_MOD_CALL);
+       bool is_call;
        u32 old_insns[LOONGARCH_LONG_JUMP_NINSNS] = {[0 ... 4] = INSN_NOP};
        u32 new_insns[LOONGARCH_LONG_JUMP_NINSNS] = {[0 ... 4] = INSN_NOP};
 
@@ -1298,6 +1299,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
        if (!is_bpf_text_address((unsigned long)ip))
                return -ENOTSUPP;
 
+       is_call = old_t == BPF_MOD_CALL;
        ret = emit_jump_or_nops(old_addr, ip, old_insns, is_call);
        if (ret)
                return ret;
@@ -1305,6 +1307,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
        if (memcmp(ip, old_insns, LOONGARCH_LONG_JUMP_NBYTES))
                return -EFAULT;
 
+       is_call = new_t == BPF_MOD_CALL;
        ret = emit_jump_or_nops(new_addr, ip, new_insns, is_call);
        if (ret)
                return ret;

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 88ad5ba7b87fd01166c9c3556d0ddf24032ab0f3..5e976730b2f5f51cfe87b3cef30860da30857ed4 100644 (file)
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -1107,8 +1107,9 @@ static void do_isync(void *info __maybe_unused)
  * execute isync (or some CSI) so that they don't go back into the
  * trampoline again.
  */
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
-                      void *old_addr, void *new_addr)
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                      enum bpf_text_poke_type new_t, void *old_addr,
+                      void *new_addr)
 {
        unsigned long bpf_func, bpf_func_end, size, offset;
        ppc_inst_t old_inst, new_inst;
@@ -1119,7 +1120,6 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
                return -EOPNOTSUPP;
 
        bpf_func = (unsigned long)ip;
-       branch_flags = poke_type == BPF_MOD_CALL ? BRANCH_SET_LINK : 0;
 
        /* We currently only support poking bpf programs */
        if (!__bpf_address_lookup(bpf_func, &size, &offset, name)) {
@@ -1132,7 +1132,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
         * an unconditional branch instruction at im->ip_after_call
         */
        if (offset) {
-               if (poke_type != BPF_MOD_JUMP) {
+               if (old_t == BPF_MOD_CALL || new_t == BPF_MOD_CALL) {
                        pr_err("%s (0x%lx): calls are not supported in bpf prog body\n", __func__,
                               bpf_func);
                        return -EOPNOTSUPP;
@@ -1166,6 +1166,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
        }
 
        old_inst = ppc_inst(PPC_RAW_NOP());
+       branch_flags = old_t == BPF_MOD_CALL ? BRANCH_SET_LINK : 0;
        if (old_addr) {
                if (is_offset_in_branch_range(ip - old_addr))
                        create_branch(&old_inst, ip, (unsigned long)old_addr, branch_flags);
@@ -1174,6 +1175,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
                                      branch_flags);
        }
        new_inst = ppc_inst(PPC_RAW_NOP());
+       branch_flags = new_t == BPF_MOD_CALL ? BRANCH_SET_LINK : 0;
        if (new_addr) {
                if (is_offset_in_branch_range(ip - new_addr))
                        create_branch(&new_inst, ip, (unsigned long)new_addr, branch_flags);

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 21c70ae3296bb59449d3ec4a60f3ccce3a9b3eea..5f9457e910e87190013f898912a8943836c4a7f6 100644 (file)
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -852,17 +852,19 @@ static int gen_jump_or_nops(void *target, void *ip, u32 *insns, bool is_call)
        return emit_jump_and_link(is_call ? RV_REG_T0 : RV_REG_ZERO, rvoff, false, &ctx);
 }
 
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
-                      void *old_addr, void *new_addr)
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                      enum bpf_text_poke_type new_t, void *old_addr,
+                      void *new_addr)
 {
        u32 old_insns[RV_FENTRY_NINSNS], new_insns[RV_FENTRY_NINSNS];
-       bool is_call = poke_type == BPF_MOD_CALL;
+       bool is_call;
        int ret;
 
        if (!is_kernel_text((unsigned long)ip) &&
            !is_bpf_text_address((unsigned long)ip))
                return -ENOTSUPP;
 
+       is_call = old_t == BPF_MOD_CALL;
        ret = gen_jump_or_nops(old_addr, ip, old_insns, is_call);
        if (ret)
                return ret;
@@ -870,6 +872,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
        if (memcmp(ip, old_insns, RV_FENTRY_NBYTES))
                return -EFAULT;
 
+       is_call = new_t == BPF_MOD_CALL;
        ret = gen_jump_or_nops(new_addr, ip, new_insns, is_call);
        if (ret)
                return ret;

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index cf461d76e9da32e3d5576de2f20deb37992ad149..a2072cabba76d9caf4f3beb0df8c3a3da4d67104 100644 (file)
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -2413,8 +2413,9 @@ bool bpf_jit_supports_far_kfunc_call(void)
        return true;
 }
 
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
-                      void *old_addr, void *new_addr)
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                      enum bpf_text_poke_type new_t, void *old_addr,
+                      void *new_addr)
 {
        struct bpf_plt expected_plt, current_plt, new_plt, *plt;
        struct {
@@ -2431,7 +2432,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
        if (insn.opc != (0xc004 | (old_addr ? 0xf0 : 0)))
                return -EINVAL;
 
-       if (t == BPF_MOD_JUMP &&
+       if ((new_t == BPF_MOD_JUMP || old_t == BPF_MOD_JUMP) &&
            insn.disp == ((char *)new_addr - (char *)ip) >> 1) {
                /*
                 * The branch already points to the destination,

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 632a83381c2d43afecbabe964d9ee65e719d34a8..b69dc7194e2c072a80137726dc613f6d874e51dc 100644 (file)
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -597,7 +597,8 @@ static int emit_jump(u8 **pprog, void *func, void *ip)
        return emit_patch(pprog, func, ip, 0xE9);
 }
 
-static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
+static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                               enum bpf_text_poke_type new_t,
                                void *old_addr, void *new_addr)
 {
        const u8 *nop_insn = x86_nops[5];
@@ -607,9 +608,9 @@ static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
        int ret;
 
        memcpy(old_insn, nop_insn, X86_PATCH_SIZE);
-       if (old_addr) {
+       if (old_t != BPF_MOD_NOP && old_addr) {
                prog = old_insn;
-               ret = t == BPF_MOD_CALL ?
+               ret = old_t == BPF_MOD_CALL ?
                      emit_call(&prog, old_addr, ip) :
                      emit_jump(&prog, old_addr, ip);
                if (ret)
@@ -617,9 +618,9 @@ static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
        }
 
        memcpy(new_insn, nop_insn, X86_PATCH_SIZE);
-       if (new_addr) {
+       if (new_t != BPF_MOD_NOP && new_addr) {
                prog = new_insn;
-               ret = t == BPF_MOD_CALL ?
+               ret = new_t == BPF_MOD_CALL ?
                      emit_call(&prog, new_addr, ip) :
                      emit_jump(&prog, new_addr, ip);
                if (ret)
@@ -640,8 +641,9 @@ out:
        return ret;
 }
 
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
-                      void *old_addr, void *new_addr)
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                      enum bpf_text_poke_type new_t, void *old_addr,
+                      void *new_addr)
 {
        if (!is_kernel_text((long)ip) &&
            !is_bpf_text_address((long)ip))
@@ -655,7 +657,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
        if (is_endbr(ip))
                ip += ENDBR_INSN_SIZE;
 
-       return __bpf_arch_text_poke(ip, t, old_addr, new_addr);
+       return __bpf_arch_text_poke(ip, old_t, new_t, old_addr, new_addr);
 }
 
 #define EMIT_LFENCE()  EMIT3(0x0F, 0xAE, 0xE8)
@@ -897,12 +899,13 @@ static void bpf_tail_call_direct_fixup(struct bpf_prog *prog)
                target = array->ptrs[poke->tail_call.key];
                if (target) {
                        ret = __bpf_arch_text_poke(poke->tailcall_target,
-                                                  BPF_MOD_JUMP, NULL,
+                                                  BPF_MOD_NOP, BPF_MOD_JUMP,
+                                                  NULL,
                                                   (u8 *)target->bpf_func +
                                                   poke->adj_off);
                        BUG_ON(ret < 0);
                        ret = __bpf_arch_text_poke(poke->tailcall_bypass,
-                                                  BPF_MOD_JUMP,
+                                                  BPF_MOD_JUMP, BPF_MOD_NOP,
                                                   (u8 *)poke->tailcall_target +
                                                   X86_PATCH_SIZE, NULL);
                        BUG_ON(ret < 0);
@@ -3985,6 +3988,7 @@ void bpf_arch_poke_desc_update(struct bpf_jit_poke_descriptor *poke,
                               struct bpf_prog *new, struct bpf_prog *old)
 {
        u8 *old_addr, *new_addr, *old_bypass_addr;
+       enum bpf_text_poke_type t;
        int ret;
 
        old_bypass_addr = old ? NULL : poke->bypass_addr;
@@ -3997,21 +4001,22 @@ void bpf_arch_poke_desc_update(struct bpf_jit_poke_descriptor *poke,
         * the kallsyms check.
         */
        if (new) {
+               t = old_addr ? BPF_MOD_JUMP : BPF_MOD_NOP;
                ret = __bpf_arch_text_poke(poke->tailcall_target,
-                                          BPF_MOD_JUMP,
+                                          t, BPF_MOD_JUMP,
                                           old_addr, new_addr);
                BUG_ON(ret < 0);
                if (!old) {
                        ret = __bpf_arch_text_poke(poke->tailcall_bypass,
-                                                  BPF_MOD_JUMP,
+                                                  BPF_MOD_JUMP, BPF_MOD_NOP,
                                                   poke->bypass_addr,
                                                   NULL);
                        BUG_ON(ret < 0);
                }
        } else {
+               t = old_bypass_addr ? BPF_MOD_JUMP : BPF_MOD_NOP;
                ret = __bpf_arch_text_poke(poke->tailcall_bypass,
-                                          BPF_MOD_JUMP,
-                                          old_bypass_addr,
+                                          t, BPF_MOD_JUMP, old_bypass_addr,
                                           poke->bypass_addr);
                BUG_ON(ret < 0);
                /* let other CPUs finish the execution of program
@@ -4020,9 +4025,9 @@ void bpf_arch_poke_desc_update(struct bpf_jit_poke_descriptor *poke,
                 */
                if (!ret)
                        synchronize_rcu();
+               t = old_addr ? BPF_MOD_JUMP : BPF_MOD_NOP;
                ret = __bpf_arch_text_poke(poke->tailcall_target,
-                                          BPF_MOD_JUMP,
-                                          old_addr, NULL);
+                                          t, BPF_MOD_NOP, old_addr, NULL);
                BUG_ON(ret < 0);
        }
 }

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2f79afe81482fe1112608003cc394f90677e0042..a9b788c7b4aa42d93b361b859d1197445de931c0 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3710,12 +3710,14 @@ static inline u32 bpf_xdp_sock_convert_ctx_access(enum bpf_access_type type,
 #endif /* CONFIG_INET */
 
 enum bpf_text_poke_type {
+       BPF_MOD_NOP,
        BPF_MOD_CALL,
        BPF_MOD_JUMP,
 };
 
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
-                      void *addr1, void *addr2);
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                      enum bpf_text_poke_type new_t, void *old_addr,
+                      void *new_addr);
 
 void bpf_arch_poke_desc_update(struct bpf_jit_poke_descriptor *poke,
                               struct bpf_prog *new, struct bpf_prog *old);

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ef4448f18aad2fa375c93b6a60eb96d9d8cbc2d5..c8ae6ab316510035f8fa7db05b83e34202e0ffb8 100644 (file)
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -3150,8 +3150,9 @@ int __weak skb_copy_bits(const struct sk_buff *skb, int offset, void *to,
        return -EFAULT;
 }
 
-int __weak bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
-                             void *addr1, void *addr2)
+int __weak bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+                             enum bpf_text_poke_type new_t, void *old_addr,
+                             void *new_addr)
 {
        return -ENOTSUPP;
 }

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 04104397c432ea9599eddf6b589777d76bc2d271..0230ad19533e9196cef8d1b2d92087e48f659253 100644 (file)
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -183,7 +183,8 @@ static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)
        if (tr->func.ftrace_managed)
                ret = unregister_ftrace_direct(tr->fops, (long)old_addr, false);
        else
-               ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, NULL);
+               ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, BPF_MOD_NOP,
+                                        old_addr, NULL);
 
        return ret;
 }
@@ -200,7 +201,10 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad
                else
                        ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
        } else {
-               ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, new_addr);
+               ret = bpf_arch_text_poke(ip,
+                                        old_addr ? BPF_MOD_CALL : BPF_MOD_NOP,
+                                        new_addr ? BPF_MOD_CALL : BPF_MOD_NOP,
+                                        old_addr, new_addr);
        }
        return ret;
 }
@@ -225,7 +229,8 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
                        return ret;
                ret = register_ftrace_direct(tr->fops, (long)new_addr);
        } else {
-               ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
+               ret = bpf_arch_text_poke(ip, BPF_MOD_NOP, BPF_MOD_CALL,
+                                        NULL, new_addr);
        }
 
        return ret;
@@ -336,8 +341,9 @@ static void bpf_tramp_image_put(struct bpf_tramp_image *im)
         * call_rcu_tasks() is not necessary.
         */
        if (im->ip_after_call) {
-               int err = bpf_arch_text_poke(im->ip_after_call, BPF_MOD_JUMP,
-                                            NULL, im->ip_epilogue);
+               int err = bpf_arch_text_poke(im->ip_after_call, BPF_MOD_NOP,
+                                            BPF_MOD_JUMP, NULL,
+                                            im->ip_epilogue);
                WARN_ON(err);
                if (IS_ENABLED(CONFIG_TASKS_RCU))
                        call_rcu_tasks(&im->rcu, __bpf_tramp_image_put_rcu_tasks);
@@ -570,7 +576,8 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
                if (err)
                        return err;
                tr->extension_prog = link->link.prog;
-               return bpf_arch_text_poke(tr->func.addr, BPF_MOD_JUMP, NULL,
+               return bpf_arch_text_poke(tr->func.addr, BPF_MOD_NOP,
+                                         BPF_MOD_JUMP, NULL,
                                          link->link.prog->bpf_func);
        }
        if (cnt >= BPF_MAX_TRAMP_LINKS)
@@ -618,6 +625,7 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link,
        if (kind == BPF_TRAMP_REPLACE) {
                WARN_ON_ONCE(!tr->extension_prog);
                err = bpf_arch_text_poke(tr->func.addr, BPF_MOD_JUMP,
+                                        BPF_MOD_NOP,
                                         tr->extension_prog->bpf_func, NULL);
                tr->extension_prog = NULL;
                guard(mutex)(&tgt_prog->aux->ext_mutex);