dynup / kpatch

kpatch - live kernel patching
GNU General Public License v2.0
1.49k stars 305 forks source link

ERROR: invalid ancestor arch/arm64/kernel/vdso/vdso.so.dbg for arch/arm64/kernel/vdso/vgettimeofday.o. #1227

Closed Conleyfree closed 2 years ago

Conleyfree commented 3 years ago

In kernel 5.10, I built a livepatch for vmlinux with kpatch, and give errors as following: ERROR: invalid ancestor arch/arm64/kernel/vdso/vdso.so.dbg for arch/arm64/kernel/vdso/vgettimeofday.o. Q1, What does this error mean?

I learn a way to resolve this probelm from this issue https://github.com/dynup/kpatch/issues/518 Then I ignored the "arch/arm64/kernel/vdso/*" in kpatch-gcc, and it works! Q2, Can I really safly ignored thie path?

joe-lawrence commented 3 years ago

Hi @Conleyfree ,

Q1: Ancestor refers to the "parent" object that a given object file is build for. For example, kpatch-build detects a change in net/netlink/af_netlink.o and then needs to know for which kernel object it belongs to. In this case it's built into vmlinux, so it proceeds to create a livepatch for vmlinux. Other cases may result in changes to a module, in which case the kernel object to be patched will be foo.ko.

Q2: Well, according to the arm64 vdso Makefile, it builds its only C file without $(CC_FLAGS_FTRACE) so nothing in the vdso is livepatchable. Ignoring the path will result in kpatch-build ignoring the changes to that file... which raises a few follow up questions:

Conleyfree commented 3 years ago

Thanks for your answer, @joe-lawrence

I used kpatch version 0.6.1 for creating livepatches. In my patch, I just add a "pr_info" statement in function "do_task_stat" at fs/proc/array.c. I'm not sure why it affects vdso files. And I tried to modify other files, and the same error was reported. This is one of my toy patch:

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 18a4588c35be..170458bbc806 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -450,6 +450,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
        unsigned long rsslim = 0;
        unsigned long flags;

+       pr_info("test klp ...\n");
        state = *get_task_state(task);
        vsize = eip = esp = 0;
        permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS | PTRACE_MODE_NOAUDIT);

According your answer, when we create a livepatch for vmlinux, it seems that arm64 vdso is not supported livepatchable. Can I submit a patch to the community to ignore arm64 vsdo?

joe-lawrence commented 3 years ago

Thanks for your answer, @joe-lawrence

I used kpatch version 0.6.1 for creating livepatches. In my patch, I just add a "pr_info" statement in function "do_task_stat" at fs/proc/array.c. I'm not sure why it affects vdso files. And I tried to modify other files, and the same error was reported. This is one of my toy patch:

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 18a4588c35be..170458bbc806 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -450,6 +450,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
        unsigned long rsslim = 0;
        unsigned long flags;

+       pr_info("test klp ...\n");
        state = *get_task_state(task);
        vsize = eip = esp = 0;
        permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS | PTRACE_MODE_NOAUDIT);

I wouldn't think that a change to fs/proc/array.c would affect the vdso, however 1) kpatch version v0.6.1 is over 3 years old and 2) upstream kpatch doesn't support the arm64 arch (patches welcome!) so I can only speculate on this.

According your answer, when we create a livepatch for vmlinux, it seems that arm64 vdso is not supported livepatchable. Can I submit a patch to the community to ignore arm64 vsdo?

We would need support for arm64 for that patch to be useful. There is some interest (see https://github.com/dynup/kpatch/issues/1216), so you may want to check in on that issue if you have patches or ideas for supporting that architecture.

Conleyfree commented 2 years ago

Thanks for your answer, @joe-lawrence I've learned these knowledge.