Skip to content

dkms.post_remove blindly removes files #521

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

Open
4 of 41 tasks
Athanasius opened this issue Jan 23, 2025 · 3 comments
Open
4 of 41 tasks

dkms.post_remove blindly removes files #521

Athanasius opened this issue Jan 23, 2025 · 3 comments

Comments

@Athanasius
Copy link

Version of xpadneo

v0.9-214-g8d20a23

Controller Model

  • Xbox One S controller
  • Xbox Elite 2 controller
  • Xbox Series X|S controller
  • Other:

Connection mode

  • Bluetooth connection
  • USB cable (not yet supported)
  • Xbox Dongle connection (not yet supported)

Installed Software

  • Anti-Micro (may affect button mappings)
  • OpenRGB (may mess up mappings and rumble stability)
  • Steam Input (enabled by default via Steam Desktop client)
  • Steam Link (usually via Raspberry Pi or other micro computers)
  • devices with QMK firmware (may affect udev rules, similar to OpenRGB)
  • netstick (shares input devices via network similar to Steam Link)
  • xboxdrv (user-space gamepad driver)
  • xone (kernel-space gamepad driver using the Xbox dongle or USB)
  • xow (alternative driver using the Xbox dongle)

Protocol Information

Please help us identify at which layer the problem can be found if you want
to report mapping errors or if the controller fails to be detected:

  • Steam Proton games are having issues
  • Steam Linux-native games are having issues
    • I don't use Steam or did not try
  • games running through Lutris, wine and/or Bottles are having issues
    • I don't use Lutris, Bottles, wine or did not try
  • Linux-native games are having issues
    • I don't use native games or did not try
  • Other software is having issues (describe software and issues below)
  • Running evtest is showing issues (describe the issues below)
    • Keep in mind that BTN_NORTH and BTN_WEST are intentionally swapped
  • Running jstest is showing issues (describe the issues below)
    • I don't have this tool or don't know how to use it
  • Running gamepad-tool is showing issues (post console output below)
    • I don't have this tool

Please describe how it is failing below in the next sections.

Severity / Impact

  • I've read the docs and the bug reporting instructions
  • I've applied the latest firmware update to the controller
  • I've tried disabling or running without above mentioned software
  • It does not work at all
  • It used to work in a previous version
  • It mostly works but sometimes it doesn't
  • I found a work-around
  • I probably didn't figure it all out but it's too early to give up
  • I don't know how to ...
  • It's too complicated
  • Fantastic work but ...
  • I can code and I want to help

Describe the Bug

dkms.post_remove is blindly removing files without checking if there's still a kernel installed that needs to make use of them. Specifically:

/etc/modprobe.d/xpadneo.conf
/etc/udev/rules.d/70-xpadneo-disable-hidraw.rules
/etc/udev/rules.d/60-xpadneo.rules

Steps to Reproduce

I use a script that first installs a new locally compiled kernel (using dpkg -i ...), and only if that succeeds do I then remove the version prior to the one currently running. i.e. I had 6.12.9 and 6.12.10 installed, am booted into 6.12.10 and have just installed 6.12.11. First 6.12.11 was installed, then 6.12.9 was removed. The latter operation invoked dkms.post_remove and removed the affected files.

Obviously if I first removed old-old version and then installed new version this would be moot, as the latter action would invoke dkms.post_install, but I have reason to do it in this order.

I only caught this because I keep /etc under git.

Expected Behavior

Some check to see if at least one DKMS applicable kernel is still installed, and if so do not remove the files.

In this instance I've worked around this by running the dkms.post_install manually to restore the files.

Perhaps the output of dkms status could be utilised for this purpose. It would be quicker to use dkms status hid-xpadneo/<version>, but I don't know if the version would easily be available to the script.

Screenshots / GIFs / Videos

System Information

# uname -a
Linux emilia 6.12.10-athan #1 SMP PREEMPT_DYNAMIC Fri Jan 17 14:25:31 GMT 2025 x86_64 GNU/Linux
# xxd -c20 -g1 /sys/module/hid_xpadneo/drivers/hid:xpadneo/0005:045E:*/report_descriptor | tee >(cksum)
<presumed irrelevant>

Controller and Bluetooth Information

Additional Context

@kakra
Copy link
Collaborator

kakra commented Jan 23, 2025

Yes, I discovered this problem lately while working on the DKMS stuff.

I've setup some instructions here:
https://github.com/atar-axis/xpadneo/blob/master/docs/PACKAGING.md

I also added a note to the release notes in Github:
https://github.com/atar-axis/xpadneo/releases/tag/v0.9.7

I think the package manager should handle this problem. DKMS doesn't have a post remove hook which only runs after the last instance is removed. And we may have conflicting changes between versions anyways.

So the package manager should take care of not allowing different versions to be installed at the same time - just different kernel instances of the same version.

Suggestions are welcome. I think we should rename PREFIX to DESTDIR, as an example.

Installing different versions of xpadneo at the same time isn't really supported - and your package manager should complain loudly about it. Of course it can only do this if you use the new installation helpers. And we should probably remove the post and pre hooks in that case?

I'm not very familiar with DKMS because I'm developing on a non-DKMS distribution. How do other projects handle this if they need to deploy udev rules?

@kakra kakra added this to the v0.10 milestone Jan 23, 2025
@Athanasius
Copy link
Author

Just to be clear, my issue is purely about uninstalling an old kernel version. I'm still using the exact same xpadneo version across all installed kernels, precisely because DKMS is taking care of that.

This is "different kernel instances of the same version":

14:52:38 0$ dkms status
hid-xpadneo/v0.9-214-g8d20a23, 6.12.10-athan, x86_64: installed
hid-xpadneo/v0.9-214-g8d20a23, 6.12.11-athan, x86_64: installed
nvidia/550.144.03, 6.12.10-athan, x86_64: installed
nvidia/550.144.03, 6.12.11-athan, x86_64: installed
v4l2loopback/0.13.2, 6.12.10-athan, x86_64: installed
v4l2loopback/0.13.2, 6.12.11-athan, x86_64: installed
virtualbox/7.1, 6.12.10-athan, amd64: installed (WARNING! Diff between built and installed module!) (WARNING! Diff between built and installed module!) (WARNING! Diff between built and installed module!)
virtualbox/7.1, 6.12.10-athan, x86_64: installed
virtualbox/7.1, 6.12.11-athan, amd64: installed
virtualbox/7.1, 6.12.11-athan, x86_64: installed

(Ignore the virtualbox gripes, it's not a "proper" dkms setup, just one I hacked together as the actual virtualbox method of installing kernel modules wasn't working 100% for me).

If I now did apt remove linux-image-6.12.10-athan then dkms would get triggered and /usr/src/hid-xpadneo-v0.9-214-g8d20a23/dkms.post_remove would get run, and thus remove the files, despite linux-image-6.12.11-athan still being installed, including the /lib/modules/6.12.11-athan/updates/dkms/hid-xpadneo.ko.xzfile.

I think the basic issue here is that the current setup is conflating "things an OS package would do" with "things DKMS does".

The installation of the modprobe and udev files should be handled at the package level, not the DKMS level. In my case the issue arises because 1) I'm using xpadneo from source, but 2) the xpadneo DKMS configuration attempts to perform these package level operations without sufficient finesse to ensure those files remain in place whilst any instance of the kernel module exists.

Obviously in the "installing from source" case there's no package management happening. On checking the Makefile install/uninstall is already handling the modprobe and udev files. So, just remove that from dkms.post_remove ? And the installation of them doesn't need to be in dkms.post_install either.

Users are either:

  1. Using a package maintainer version of xpadneo, in which case that should take care of the modprobe and udev files, and there's no need for the dkms configuration to attempt it.
  2. Using xpadneo from source, in which case make install will install the modprobe and udev files and make uninstall will remove them, and again there's no need for the dkms configuration to attempt it.

I see this behaviour of dkms.post_remove was first introduced in 38a8d43 (Mon May 21 00:25:07 2018 +0200) but the commit message doesn't explain why (no issue or other reference). Similar removal of modprobe and udev files was added to Makefile in b418e9f (Wed Dec 18 22:59:53 2024 +0100). So, perhaps the former commit should be reconsidered now ?

@kakra
Copy link
Collaborator

kakra commented Jan 24, 2025

Yes, during testing I removed the hooks from the DKMS config file. But this introduced some unwanted problems. I think we should reconsider that before tagging v0.10.

I still think that, when installing from source, xpadneo should not try to be smart about keeping or not keeping the files. Rather, DKMS should do the right thing and provide separate hooks for the initial installation and final uninstallation. I don't want to replicate functionality which DKMS could easily provide for a whole community.

We should consider the following:

  • Installation of an instance for the current kernel will require the matching etc files to be installed: This means that installing different xpadneo versions side by side is not supported.
  • The dkms remove action, according to man dkms should probably not be called by kernel removal - rather dkms uninstall with the kernel version must be called: dkms uninstall should not call the dkms.post_remove hook.
  • The dkms.post_install hook is called to update or install the etc files with the matching version.

This makes me think that xpadneo is properly behaving:

  • We use dkms.post_remove to remove xpadneo including etc files - that's what has been asked of us: "To completely remove a driver, the remove action should be utilized".
  • Kernel packages must not call dkms remove but dkms uninstall instead.

If this still triggers the wrong actions, it's either a bad kernel package script, DKMS is behaving wrong, or DKMS docs are wrong. I'd go even further and say that kernel package script should probably use dkms build/unbuild - because that's exactly the smallest change they need to do.

A symmetric action would be to use dkms.post_add instead of dkms.post_install but that would only make your situation worse because installing for a kernel version would no longer reinstall the etc files that may have been previously purged accidentally. I think this also makes a worse user experience. We have a lot reports about weird DKMS edge cases, leading to stalled/hanging boots sometimes - which is very bad.

But maybe I am understanding DKMS completely wrong. OTOH, I have a gut feeling of DKMS not implementing a proper separation of concerns since some time now. It also doesn't properly catch all errors and/or doesn't bubble them up to the caller.

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

No branches or pull requests

2 participants