dynup / kpatch

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

function __pfx_new_sync_read has no fentry/mcount call, unable to patch #1385

Closed joe-lawrence closed 4 months ago

joe-lawrence commented 7 months ago

Xinhua Li reports that building the following kpatch with RHEL9 kernel-5.14.0-362.24.1.el9_3.x86_64:

--- linux-5.14.0-362.24.1.el9.x86_64/fs/read_write.c 2024-02-15 19:12:01.000000000 +0800
+++ linux-5.14.0-362.24.1.el9.x86_64.kp/fs/read_write.c 2024-04-11 20:00:46.774995241 +0800
@@ -470,6 +470,8 @@ ssize_t vfs_read(struct file *file, char
    if (unlikely(!access_ok(buf, count)))
        return -EFAULT;

+   printk(KERN_ALERT "current task is %s\n", current->comm);
+
    ret = rw_verify_area(READ, file, pos, count);
    if (ret)
        return ret;

results in a kpatch-build error:

Extracting new and modified ELF sections
ERROR: read_write.o: 1 function(s) can not be patched
create-diff-object: unreconcilable difference
read_write.o: function __pfx_new_sync_read has no fentry/mcount call, unable to patch
ERROR: 1 error(s) encountered. Check /root/.kpatch/build.log for more details.
joe-lawrence commented 7 months ago

repro-1385.tar.gz

I believe the problem is that:

  1. new_sync_read() and __pfx_new_sync_read() functions move from ELF section .text.new_sync_read to .text.unlikely.new_sync_read
  2. kpatch_compare_correlated_symbol() allows for such movement, marking both new_sync_read and __pfx_new_sync_read symbols as CHANGED
  3. kpatch_find_func_profiling_calls() skips __pfx_new_sync_read since is a function "prefix"
  4. kpatch_check_func_profiling_calls() checks that all CHANGED STT_FUNC symbols have an fentry/mcount call, including __pfx_new_sync_read which fails because of (3)

Currently testing a fix to amend (4) to skip the fentry/mcount tests on prefix functions.

joe-lawrence commented 7 months ago

If we want such _pfx symbols marked as CHANGED (I think that would be consistent), then kpatch_create_patches_sections() will also have to gloss over them to exclude them from the set of kpatched functions.

See potential fix: https://github.com/joe-lawrence/kpatch/commit/56152d12b911072d6ecb74607ae74bfafb8e7b98

github-actions[bot] commented 6 months ago

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

github-actions[bot] commented 6 months ago

This issue was closed because it was inactive for 7 days after being marked stale.

github-actions[bot] commented 5 months ago

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

github-actions[bot] commented 4 months ago

This issue was closed because it was inactive for 7 days after being marked stale.