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

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

Closed
joe-lawrence opened this issue Apr 12, 2024 · 6 comments
Closed
Labels

Comments

@joe-lawrence
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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: joe-lawrence@56152d1

Copy link

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 github-actions bot added the stale label May 16, 2024
Copy link

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

Copy link

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 github-actions bot added the stale label Jun 23, 2024
Copy link

github-actions bot commented Jul 1, 2024

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

@github-actions github-actions bot closed this as completed Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant