Conversation
While vmmon module already uses explicit module_init() and module_exit() for its init and cleanup function, vmnet relies on traditional magic names init_module() and cleanup_module(). Apparently this has an unfortunate side effect that the two functions are not identified as indirect call targets by objdump and they get "sealed" when the module is built against and loaded into an IBT enabled kernel. Starting with 6.3-rc1, objtool is going to warn about this issue, indicating that the legacy module initialization is deprecated and module_init() and module_exit() macros should be used instead so do that for vmnet as well.
Some cross page functions need an explicit endbr64 instruction as they are indirect branch targets but are not recognized as such. VMware 17 uses home cooked ENDBR macro rather than standard ASM_ENDBR defined in kernel. Use ASM_ENDBR instead and define it as empty if not available (kernel before 5.18-rc1) so that we do not generate useless endbr64 instructions when building against kernel which does not support IBT or has it disabled.
Mainline commit 6308499b5e99 ("net: unexport csum_and_copy_{from,to}_user")
in 5.19-rc1 unexports csum_and_copy_to_user as no in-tree module is using
it. A clean solution would probably be rewriting the code to use iovec
iterator as csum_and_copy_to_iter() is still exported (or perhaps
skb_copy_and_csum_datagram() might be used instead). Anything like this
would be way too intrusive so it would have to wait for VMware developers.
Workstation 17.0.0 handles this with a call to csum_partial_copy_nocheck()
inside a user_access_begin()/user_access_end() block which lets the build
succeed but objtool still warns about a call to csum_partial_copy_nocheck()
with UACCESS enabled. Based on the reasoning in commit message of mainline
commit ea24213d8088 ("objtool: Add UACCESS validation"), this workaround
does indeed seem questionable.
Use the older workaround combining csum_partial() with copy_to_user() like
in workstation-16.2.4 branch instead. This will be less efficient but
hopefully the performace hit will not be noticeable.
Mainline commit c304eddcecfe ("net: wrap the wireless pointers in struct
net_device in an ifdef") in 5.19-rc1 makess ieee80211_ptr member present in
struct net_device only if CONFIG_CFG80211 is enabled. Workstation 17.0.0
checks if CONFIG_CFG80211 is enabled but only when CONFIG_WIRELESS_EXT is
not. Thus a build against a kernel with CONFIG_WIRELESS_EXT enabled and
CONFIG_CFG80211 disabled still fails. Also, the newly introduced version
check is pointless, there is no point checking dev->ieee80211_ptr with
CONFIG_CFG80211 disabled (fortunately it will be null anyway).
Rewrite the check in VNetBridgeIsDeviceWireless() to check each of the two
pointers only if it actually exists.
Two functions use "foo()" rather than proper "foo(void)" in their definitions and as reported, clang compiler treats it as an error. While at it, also mark VNetFreeInterfaceList() static to make its definition match the declaration.
This also requires tweaking the Makefiles to allow for building for non-active kernels
|
I still get the compilation error vmmon.o: warning: objtool: CrossPage_CodePage+0x1d6: corrupt function pointer reference With the 6.13.9-200.fc41.x86_64 kernel. In fact I've been getting that corrupt function pointer reference problem with earlier kernels as well but I didn't track them. Can anyone else reproduce this using this branch? |
I'm not sure how far back this change is compatible. Maybe someone will let me know.
|
Worked great in Fedora 42 with kernel 6.15.3-200.fc42.x86_64 without errors! Thanks! |
|
Hello. I probably did something wrong, but when I apply this, my VM network will stop working. |
|
@zDEFz Does VM network work fine when you use #306 (comment) ? Edit: |
Sorry, but I think this is a displaced question. |
|
Well, I asked so that we can better pinpoint if there is an issue in this PR, or in your setup in general. Also about reviewability / transparency about made changes, I feel like a PR with 76k LOC change (like this one) is not much better, optimally we'd have a PR that just shows the diff (what changes needed to be done so it compiles) or am I missing something? |
I just see, that @philipl did something that leads to the actual changes he made to be shown at the bottom of the commit history. I made a branch from the changes of @jeamieofqidan for 6.15 here: |
How does that differ from the version that works from kernel 6.16? |
|
This PR is structured to create a branch for 17.6 that follows the pattern used for creating branches for older releases. As a result it's not going to be just the changes required to fix compilation. If you look at the source branch for the PR itself, the history is clear. |
You'd need to take the tgz for 6.16 and overwrite all files, then commit the changes. This way you'd get a commit which contains the diff, to see what jeamieofqidan changed for 6.16 compatibility. I might do this later if I find the time for it.
But why the latest commits show up as first commits? It does not make intuitive sense. I think it should be possible to add your newer commits afterward, not before. Why is it done this way? |
I don't know what you mean. github decides how to show the commits in a PR, but they are in ascending order on my screen, with the newest commits at the bottom of the list. |
not for me. what do you see? |
|
That screenshot isn't from this PR's commit list, so I don't know what you're looking at. |
It's from the link that was posted https://github.com/powellnorma/vmware-host-modules/commits/workstation-17.6.3_jeamieofqidan/ What link did you visit? |
|
The commits for this PR. That we are adding comments to. The link you are referring to is the commit list for a branch, and github shows a branch's commit history in descending order with the most recent first. That's normal git behaviour. |
What I meant were the oldest commits, I got confused why they were shown at the beginning of the PR commit history. For new PRs, GH seems to by default base them off from the repos master branch. What one can do as contributor though, is to chose as base branch the latest available one, instead of master. |
|
Added the additional patch needed for >= 6.16 |
|
Updated - works and tested. Thanks @philipl https://aur.archlinux.org/packages/vmware-host-modules-dkms-fix-git |
8 participants
Very very minor changes to some of the RHEL support macros. This works with 6.13.x and 6.14.x kernels.
Edit: Updated with support for 6.15.x and 6.16.x
Edit: 17.6.4 is identical.
Edit: Kernel 6.17.x doesn't require any additional changes (surprise!)