Compile fixes for kernel 5.15.2 #1
Conversation
Linux 5.15.2 updated security.h security_binder_* functions (like security_binder_transfer_binder(), so it uses struct cred* instead of struct task_struct*)
|
Works good, tested in a clean install |
|
Works good |
|
Thanks a lot for your PR...! I didn't even see it at first because my notifications were not correctly set up :-( ... I will have a look at it this weekend. |
|
@modscleo4 great PR! I think |
binder/binder.c
Outdated
| goto done; | ||
| } | ||
| #if (LINUX_VERSION_CODE >= KERNEL_VERSION(5,15,2)) | ||
| if (security_binder_transfer_binder(proc->tsk->real_cred, target_proc->tsk->real_cred)) { |
There was a problem hiding this comment.
Looking at the most recent version of binder_translate_binder() in the kernel (see this link), the function is invoked slightly differently:
if (security_binder_transfer_binder(proc->cred, target_proc->cred)) {
This differs from your code. It is possible that your code works nonetheless, but I would like to keep binder.c in the modules here as consistent as possible to the version in the upstream kernel. So, could you please change this line accordingly? Please also check the "security_binder_*" calls that you changed below to make sure that the code matches as much as possible to "binder.c" in the kernel.This makes the modules here much easier to maintain.
There was a problem hiding this comment.
Right. Updating as soon as I can
There was a problem hiding this comment.
I changed to be in same style as Android binder.c, and I noticed there's more changes in the binder.c and struct binder_proc (new fields). A new PR may be needed
Indeed, it seems that the same changes are also required on this kernel. It seems like this patch was backported to 5.10 and probably also to a plethora of other kernel versions then; probably it's a bugfix or fixes a security problem. But given how many kernel versions are going to receive this fix, it will become increasingly difficult to write a kernel version check using LINUX_VERSION_CODE that reliably verifies whether that kernel has the change or not... But I think we have to try; the only other option would be to create a new branch with the old code and to make an incompatible change on the master branch, which only works on kernels that have this bugfix. |
Good point— I honestly think it could be easier just keeping it in just one version, perhaps just tag new releases rather than support the old releases. It might get messy? @choff |
|
@choff this woks good on all of the following kernels (all except binder on 5.10-lts): |
|
@modscleo4 can you check: modscleo4#1 |
|
Hello @sickcodes and @modscleo4 , thanks for the PR and for testing it! I have now merged your code. The only thing I still want to do is to remove some unused fields in the "struct binder_proc", which @modscleo4 also added (e.g. "outstanding_txns"), but I will do that in a separate commit. And then there's also the issue with the latest 5.10 kernel still to be fixed. For that, we'd need to improve the version checking as I really wouldn't like to introduce an incompatibility at this point. I will try to also look into that. Best regards, Christian |
oh sorry I only meant to add the |
|
Hey @choff here was the 5.10 fix: modscleo4#1 |
|
Removing the ifdefs works on latest kernel and lts 😋 I opened a PR on @modscleo4's fork, but it works without the ifdefs. I think just tag pre commit as 5.14, and remove the ifdefs, should be fine. |
|
Works for 5.15.6 but fails for lts (master branch) |
|
@04ELY use my PR fork, works on 5.10 |
It's fixed now for 5.10 in the AUR. I've tested on 5.10 thru 5.14. Let me know if you have any issues! |
* removed obsolete name="%k", added binder symlink * removed obsolete name="%k", added binder symlink
Linux 5.15.2 updated security.h security_binder_* functions (like security_binder_transfer_binder(), so it uses struct cred* instead of struct task_struct*)