Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kpatch-build: rela section could disappear after patched #1301

Merged
merged 1 commit into from Sep 23, 2022

Conversation

anatasluo
Copy link
Contributor

@anatasluo anatasluo commented Sep 15, 2022

Hi, guys. I meet a segment fault when I use the Kpatch.

My patch is

From d4238410bab5b907ee514c15ab4d9109d6da2796 Mon Sep 17 00:00:00 2001
From: Luo Longjun <luolongjuna@gmail.com>
Date: Wed, 14 Sep 2022 16:38:28 +0800
Subject: [PATCH] kpatch test

---
 fs/proc/version.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/proc/version.c b/fs/proc/version.c
index b449f186577f..734ddfc7d316 100644
--- a/fs/proc/version.c
+++ b/fs/proc/version.c
@@ -6,12 +6,8 @@
 #include <linux/seq_file.h>
 #include <linux/utsname.h>
 
-static int version_proc_show(struct seq_file *m, void *v)
+notrace noinline static int version_proc_show(struct seq_file *m, void *v)
 {
-	seq_printf(m, linux_proc_banner,
-		utsname()->sysname,
-		utsname()->release,
-		utsname()->version);
 	return 0;
 }
 
-- 
2.37.3

My test environment:

NAME="Fedora Linux"
VERSION="36 (Server Edition)"
ID=fedora
VERSION_ID=36
VERSION_CODENAME=""
PLATFORM_ID="platform:f36"
PRETTY_NAME="Fedora Linux 36 (Server Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:36"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f36/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=36
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=36
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Server Edition"
VARIANT_ID=server

The output of the Kpatch:

WARNING: Skipping compiler version matching check (not recommended)
Using cache at /home/anatasluo/.kpatch/src
Testing patch file(s)
Reading special section data
Building original source
Building patched source
Extracting new and modified ELF sections
/home/anatasluo/Git/kpatch/kpatch-build/kpatch-build: line 1082: 2901326 Segmentation fault      (core dumped) "$TOOLSDIR"/create-diff-object $CDO_FLAGS "orig/$i" "patched/$i" "$KOBJFILE_NAME" "$SYMTAB" "$SYMVERS_FILE" "${MODNAME//-/_}" "output/$i" 2>&1
     2901327 Done                    | logger 1
ERROR: create-diff-object SIGSEGV
ERROR: There was a SIGSEGV, but no core dump was found in the current directory.  Depending on your distro you might find it in /var/lib/systemd/coredump or /var/crash.. Check /home/anatasluo/.kpatch/build.log for more details.

After patched, the function of 'version_proc_show' has no rela entries. So it has no corresponding rela section.
In common cases, each function will have at least two rela entries in X86. One is from trace mechanism, this rela entry can be removed by the 'notrace' macro. Another is from the return thunk mechanism, make 'CONFIG_RETHUNK=n' can remove it. Fedora 36 disables this config. So this problem mainly can appear in fedora36.

@yhcote yhcote mentioned this pull request Sep 15, 2022
@anatasluo anatasluo changed the title upatch-build: rela section could disappear after patched kpatch-build: rela section could disappear after patched Sep 21, 2022
After patched, rela information for some sections could
disappear. For example, a function like the following:

"
notrace noinline static int version_proc_show(struct seq_file *m,
    void *v)
{
    return 0;
}
"

Apart from common rela entries, trace and return thunk mechanism
will generate rela information. Use `notrace` to remove the
effect of trace. Make CONFIG_RETHUNK=n can remove the effect of
return thunk.

Discovered-by: Zongwu Li <lizongwu@huawei.com>
Signed-off-by: Longjun Luo <luolongjuna@gmail.com>
@joe-lawrence
Copy link
Contributor

I can reproduce using a similar patch, using noreturn when CONFIG_RETHUNK is set to eliminate the __x86_return_thunk relocation:

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 0d2514b4f..fd14af3a5 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5299,28 +5299,9 @@ bool freeze_workqueues_busy(void)
  * CONTEXT:
  * Grabs and releases wq_pool_mutex, wq->mutex and pool->lock's.
  */
-void thaw_workqueues(void)
+__noreturn notrace void thaw_workqueues(void)
 {
-       struct workqueue_struct *wq;
-       struct pool_workqueue *pwq;
-
-       mutex_lock(&wq_pool_mutex);
-
-       if (!workqueue_freezing)
-               goto out_unlock;
-
-       workqueue_freezing = false;
-
-       /* restore max_active and repopulate worklist */
-       list_for_each_entry(wq, &workqueues, list) {
-               mutex_lock(&wq->mutex);
-               for_each_pwq(pwq, wq)
-                       pwq_adjust_max_active(pwq);
-               mutex_unlock(&wq->mutex);
-       }
-
-out_unlock:
-       mutex_unlock(&wq_pool_mutex);
+       while(1);
 }
 #endif /* CONFIG_FREEZER */

which results in the relocation section going away:

$ readelf --wide --sections ~/.kpatch/tmp/orig/kernel/workqueue.o | grep thaw_workqueues
  [290] .text.thaw_workqueues PROGBITS        0000000000000000 006f60 00008e 00  AX  0   0 16
  [291] .rela.text.thaw_workqueues RELA            0000000000000000 084578 000138 18   I 609 290  8

$ readelf --wide --sections ~/.kpatch/tmp/patched/kernel/workqueue.o | grep thaw_workqueues
  [290] .text.thaw_workqueues PROGBITS        0000000000000000 006f60 000002 00  AX  0   0 16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants