On Sun, Jul 4, 2021 at 7:19 AM Alexei Starovoitov
[off-list ref] wrote:

quoted

quoted

+                     struct bpf_insn ld_addrs[2] = {
+                             BPF_LD_IMM64(BPF_REG_3, (long)prog),

The "prog" pointer value is used here.

quoted

+                     };
+
+                     insn_buf[0] = ld_addrs[0];
+                     insn_buf[1] = ld_addrs[1];
+                     insn_buf[2] = *insn;
+                     cnt = 3;
+
+                     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;

After bpf_patch_insn_data(), a new prog may be allocated.
Is the above old "prog" pointer value updated accordingly?
I could have missed something.

excellent catch. The patching of prog can go bad either here
or later if patching of some other insn happened to change prog.
I'll try to switch to dynamic prog fetching via ksym.
The timers won't work in the interpreted mode though.
But that's better trade-off than link-list of insns to patch with a prog
after all of bpf_patch_insn_data are done?
Some other way to fix this issue?

fyi we've discussed it with Martin offline and he came up with an excellent idea
to patch prog->aux instead of prog here. I'll add that to a respin.