Allow dkms to work in a chroot environment without /proc #485

akorn · 2025-01-28T13:33:27Z

I didn't author these changes; I'm just submitting the pull request (with the author's permission) in the hope that it can be merged with little or no discussion.

It's a fairly small and self-explanatory set of changes that, at least at first glance, shouldn't alter existing behaviour, just make dkms as a whole more robust by not requiring /proc to be mounted for it to work.

scaronni · 2025-01-29T07:01:31Z

@akorn I'm all for the change, but the tests are failing, can you have a look? Thanks.

If the change requires changes to the tests, please provide those as well.

akorn · 2025-01-31T17:36:49Z

@scaronni, with my latest commit the tests mostly succeed, except this one:

Building and installing the noisy test module
--- test_cmd_expected_output.log        2025-01-31 17:29:58.377860489 +0000
+++ test_cmd_output.log 2025-01-31 17:30:09.022193710 +0000
@@ -17,9 +17,7 @@
 pre_build: line 3
 pre_build: line 4/stderr
 pre_build: line 5
-Cleaning build area...(bad exit status: 2)
-Failed command:
-make clean
+Cleaning build area... done.
 Building module(s)... done.
 Signing module /var/lib/dkms/dkms_noisy_test/1.0/build/dkms_noisy_test.ko
 Running the post_build script:
@@ -29,9 +27,7 @@
 post_build: line 3
 post_build: line 4/stderr
 post_build: line 5
-Cleaning build area...(bad exit status: 2)
-Failed command:
-make clean
+Cleaning build area... done.
 Running the pre_install script:
 /var/lib/dkms/dkms_noisy_test/1.0/source/script.sh pre_install
 pre_install: line 1
Error: unexpected output from: dkms install -k 6.11.10-amd64 -m dkms_noisy_test -v 1.0

It seems that where the test expects make clean to fail, it now succeeds. :)

Is that really a problem?

If yes, I don't know how to fix it (i.e. break it again); I couldn't immediately understand how the tests even work.

akorn · 2025-02-04T09:12:41Z

@anbe42, you wrote that noisy test that now succeeds instead of failing. Can you help a bit here? Why/how is make clean supposed to fail? And do you have an idea why it succeeds in my chroot instead?

anbe42 · 2025-02-04T12:18:15Z

Do you have the same kernel running on the host and installed in the chroot? Try a different kernel in the chroot.

Usually host and chroot have different kernels (as it happens in all the CI scenarios) and the wrong 'make clean' command picks the wrong kernel (the one from the host). IIRC I had some checks for this issue, but maybe not in this specific test case.

akorn · 2025-02-04T12:48:35Z

Do you have the same kernel running on the host and installed in the chroot? Try a different kernel in the chroot.

If I only have a different kernel in the chroot, the tests fail much sooner:

Using kernel 6.11.10-amd64/x86_64
Checking module compression ...
grep: /lib/modules/6.11.10-amd64/build/.config: No such file or directory
config: 
files: 
Expected extension: (none)
Preparing a clean test environment
Test that there are no dkms modules installed
Test framework file hijacking
*** Testing dkms on a regular module
Adding the test module by version (expected error)
Adding the test module by directory
Adding the test module again (expected error)
Adding the test module by version (expected error)
Building the test module
Error: command 'dkms build -k 6.11.10-amd64 -m dkms_test -v 1.0' returned status 21 instead of expected 0

Error! Your kernel headers for kernel 6.11.10-amd64 cannot be found at /lib/modules/6.11.10-amd64/build or /lib/modules/6.11.10-amd64/source.
Please install the linux-headers-6.11.10-amd64 package or use the --kernelsourcedir option to tell DKMS where it's located.

If I have both the same and a newer kernel, the testsuite fails later:

Running dkms autoinstall with multiple modules
--- test_cmd_expected_output.log        2025-02-04 12:33:31.706315727 +0000
+++ test_cmd_output.log 2025-02-04 12:34:00.402266447 +0000
@@ -24,9 +24,7 @@
 pre_build: line 3
 pre_build: line 4/stderr
 pre_build: line 5
-Cleaning build area...(bad exit status: 2)
-Failed command:
-make clean
+Cleaning build area... done.
 Building module(s)... done.
 Signing module /var/lib/dkms/dkms_noisy_test/1.0/build/dkms_noisy_test.ko
 Running the post_build script:
@@ -36,9 +34,7 @@
 post_build: line 3
 post_build: line 4/stderr
 post_build: line 5
-Cleaning build area...(bad exit status: 2)
-Failed command:
-make clean
+Cleaning build area... done.
 Running the pre_install script:
 /var/lib/dkms/dkms_noisy_test/1.0/source/script.sh pre_install
 pre_install: line 1
Error: unexpected output from: dkms autoinstall -k 6.11.10-amd64

And if I have an older, the same, and a newer kernel:

*** Testing more dkms features
Adding test module with patches
Building and installing the test module with patches
Unbuilding the test module with patches
Adding test module with pre/post scripts
Building and installing the test module with pre/post scripts
Unbuilding the test module with pre/post scripts
Adding noisy test module
Building and installing the noisy test module
Sign command: /lib/modules/6.11.10-amd64/build/scripts/sign-file
Signing key: /var/lib/dkms/mok.key
Public certificate (MOK): /var/lib/dkms/mok.pub

applying patch patch2.patch...patching file Makefile
patching file dkms_noisy_test.c
 done.
applying patch patch1.patch...patching file Makefile
Hunk #1 succeeded at 3 (offset 2 lines).
patching file dkms_noisy_test.c
Hunk #1 succeeded at 18 (offset 2 lines).
 done.
Running the pre_build script:
/var/lib/dkms/dkms_noisy_test/1.0/build/script.sh pre_build
pre_build: line 1
pre_build: line 2/stderr
pre_build: line 3
pre_build: line 4/stderr
pre_build: line 5
Cleaning build area... done.
Building module(s)... done.
Signing module /var/lib/dkms/dkms_noisy_test/1.0/build/dkms_noisy_test.ko
Running the post_build script:
/var/lib/dkms/dkms_noisy_test/1.0/build/script.sh post_build
post_build: line 1
post_build: line 2/stderr
post_build: line 3
post_build: line 4/stderr
post_build: line 5
Cleaning build area... done.
Running the pre_install script:
/var/lib/dkms/dkms_noisy_test/1.0/source/script.sh pre_install
pre_install: line 1
pre_install: line 2/stderr
pre_install: line 3
pre_install: line 4/stderr
pre_install: line 5
Installing /lib/modules/6.11.10-amd64/updates/dkms/dkms_noisy_test.ko.xz
Running the post_install script:
/var/lib/dkms/dkms_noisy_test/1.0/source/script.sh post_install
post_install: line 1
post_install: line 2/stderr
post_install: line 3
post_install: line 4/stderr
post_install: line 5
Running depmod... done.
Unbuilding the noisy test module
*** Testing multiple dkms modules
Adding test module with dependencies
Adding build-exclusive test module
Adding noautoinstall test module
Adding test module
Running dkms autoinstall with multiple modules
--- test_cmd_expected_output.log        2025-02-04 12:44:48.733996280 +0000
+++ test_cmd_output.log 2025-02-04 12:45:17.330355693 +0000
@@ -24,9 +24,7 @@
 pre_build: line 3
 pre_build: line 4/stderr
 pre_build: line 5
-Cleaning build area...(bad exit status: 2)
-Failed command:
-make clean
+Cleaning build area... done.
 Building module(s)... done.
 Signing module /var/lib/dkms/dkms_noisy_test/1.0/build/dkms_noisy_test.ko
 Running the post_build script:
@@ -36,9 +34,7 @@
 post_build: line 3
 post_build: line 4/stderr
 post_build: line 5
-Cleaning build area...(bad exit status: 2)
-Failed command:
-make clean
+Cleaning build area... done.
 Running the pre_install script:
 /var/lib/dkms/dkms_noisy_test/1.0/source/script.sh pre_install
 pre_install: line 1
Error: unexpected output from: dkms autoinstall -k 6.11.10-amd64

It doesn't seem like I can satisfy this particular test. @anbe42, any suggestions?

akorn · 2025-02-06T19:07:04Z

@scaronni meanwhile, can you perhaps rerun the tests in the official test environment? Who knows, they might succeed there. :) Thanks!

anbe42 · 2025-02-07T13:31:25Z

The applied substitution does not produce identical results if it involves empty command output:

#!/bin/bash

f()
{
echo foo
}

g()
{
:
}

while read line; do
        echo "< <(f): '$line'"
done < <(f)
while read line; do
        echo "< <(g): '$line'"
done < <(g)

while read line; do
        echo '<<< "$(f)":' "'$line'"
done <<< "$(f)"
while read line; do
        echo '<<< "$(g)":' "'$line'"
done <<< "$(g)"

< <(f): 'foo'
<<< "$(f)": 'foo'
<<< "$(g)": ''

This breaks e.g. 'autoinstall'

*** Testing dkms autoinstall/kernel_{postinst/prerm}, dkms_autoinstaller
Testing without modules and without headers
 Running dkms autoinstall
Error: command 'dkms autoinstall -k 6.12.8-amd64-noheaders' returned status 1 instead of expected 0
/home/anbe/deb-src/dkms/dkms/dkms: line 2535: latest: bad array subscript
/home/anbe/deb-src/dkms/dkms/dkms: line 2537: latest[$m]: bad array subscript

Please rebase your branch on master.

scaronni · 2025-02-10T09:08:08Z

@akorn i've also fixed the issues with the fedora rawhide containers, so if you rebase on master at least you would get working tests.

akorn · 2025-02-10T09:48:21Z

The applied substitution does not produce identical results if it involves empty command output:

I know. I was hunting for occurrences where this mattered and already resolved some of them in commits 6801dc1 and 5f9f3c0, but not all (not being able to run the tests myself doesn't help -- I guess I'll just disable the noisy one locally).

*** Testing dkms autoinstall/kernel_{postinst/prerm}, dkms_autoinstaller
Testing without modules and without headers
Running dkms autoinstall
Error: command 'dkms autoinstall -k 6.12.8-amd64-noheaders' returned status 1 instead of expected 0
/home/anbe/deb-src/dkms/dkms/dkms: line 2535: latest: bad array subscript
/home/anbe/deb-src/dkms/dkms/dkms: line 2537: latest[$m]: bad array subscript

I'll deal with this soon.

Please rebase your branch on master.

Done.

akorn · 2025-02-10T15:37:14Z

@scaronni, can you rerun the tests? They succeed for me now (except the noisy one, but I don't think that's related to my changes).

scaronni · 2025-02-11T13:27:06Z

Running, sorry for the delay.

anbe42 · 2025-02-11T13:49:09Z

it now works for me in my Debian test setup

I'll try to test without /proc, too

@scaronni: is it possible to add one test (any distribution, prefereably one utilizing the weak module code paths) with /proc unmounted to the CI matrix?

@akorn: please rebase (git rebase) on master, don't merge master into the topic branches

scaronni · 2025-02-11T15:17:16Z

@scaronni: is it possible to add one test (any distribution, prefereably one utilizing the weak module code paths) with /proc unmounted to the CI matrix?

I think we can just add an extra tag in the table here: https://github.com/dell/dkms/blob/master/.github/workflows/tests.yml#L15

And then trigger a specific task in the tests than unmouns /proc. Maybe even better, shall we just add a test that unmounts /proc for all distributions in run_test.sh?

@akorn: please rebase (git rebase) on master, don't merge master into the topic branches

Yes please, do a clean rebase (Fast Forward merge), no merge commits please. Besides this, I think it's fine for merging once corrected. Thanks.

scaronni · 2025-02-12T13:32:35Z

@anbe42 I'm fine with merging as soon as the tests pass. Do you agree?

dkms.in

anbe42

In bash we can use [[ $var ]] to check for non-empty values. While it is not being used consistently, [[ -n "$var" ]] is less common (8 vs. 13 occurrences). So I'd prefer the shorter form.

akorn · 2025-02-13T10:39:33Z

In bash we can use [[ $var ]] to check for non-empty values. While it is not being used consistently, [[ -n "$var" ]] is less common (8 vs. 13 occurrences). So I'd prefer the shorter form.

Should I make this change everywhere for consistency, or just in the new lines that I'm adding?

anbe42 · 2025-02-13T10:43:22Z

Should I make this change everywhere for consistency, or just in the new lines that I'm adding?

only in your new bits in order not to interfere with the ongoing shellcheck changes (#492)

akorn · 2025-02-13T13:54:43Z

Should I make this change everywhere for consistency, or just in the new lines that I'm adding?

only in your new bits in order not to interfere with the ongoing shellcheck changes (#492)

Done, I think.

anbe42 · 2025-02-13T14:04:56Z

It seems that where the test expects make clean to fail, it now succeeds. :)

I finally managed to reproduce that and I'm currently testing a fix ...

anbe42 · 2025-02-13T14:08:34Z

it now works for me in my Debian test setup

I'll try to test without /proc, too

need to try that again, my machine crashed last night ... hopefully unrelated to that test ;-)

anbe42

I think you can squash most of your fixup commits together. The /proc/cpuinfo check should stay separate. If I'm not mistaken, everything else deals with skipping the empty lines introduced by using the alternate idiom.

Please wait with rebasing for #492 (the shellcheck changes) being merged, that may cause some conflicts and may require some further reworking.

I've just tried adding a umount /proc && run_tests.sh test to the Debian package and tested your branch with it. Didn't explode ;-)

dkms.in

akorn · 2025-02-17T14:26:42Z

I think you can squash most of your fixup commits together. The /proc/cpuinfo check should stay separate. If I'm not mistaken, everything else deals with skipping the empty lines introduced by using the alternate idiom.

Yes, I think so too.

Please wait with rebasing for #492 (the shellcheck changes) being merged, that may cause some conflicts and may require some further reworking.

Roger that, standing by.

I've just tried adding a umount /proc && run_tests.sh test to the Debian package and tested your branch with it. Didn't explode ;-)

That's good news -- I think I fixed almost all bugs that could have led to actual explosions. :)

scaronni · 2025-02-22T10:27:14Z

@akorn I'm back from holiday, #492 has been merged, please rebase. Thanks.

akorn · 2025-02-23T09:53:55Z

OK, after, ah, gaining some new experience with git I believe this is now done.

NB. when I first ran the tests locally, there was a cosmetic difference in make.log output (a blank line was missing that the test expected), but I couldn't reproduce this on subsequent runs.

scaronni · 2025-02-24T08:24:41Z

New shell check errors:

Run statuscode=0
./dkms.in:829:12: warning: Expanding an array without an index only gives the first element. [SC2128]
./dkms.in:1992:15: note: read without -r will mangle backslashes. [SC2162]
./dkms.in:1994:24: note: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns. [SC2295]
./dkms.in:2026:15: note: read without -r will mangle backslashes. [SC2[162](https://github.com/dell/dkms/actions/runs/13481860275/job/37696175820?pr=485#step:3:169)]
./dkms.in:2442:15: note: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. [SC2116]
./dkms.in:2582:17: note: read without -r will mangle backslashes. [SC2162]
Run exit 1

akorn · 2025-02-24T13:06:32Z

@scaronni, can you merge this, or should I squash the last commit (fix shellcheck warnings) into the previous one (avoid running loops on empty strings)?

scaronni · 2025-02-24T13:55:20Z

For me is fine as is. @anbe42 ?

anbe42 · 2025-02-26T13:41:38Z

@scaronni: I've tried adding tests doing umount /proc, but that does not work. Most likely because we have UID 0 but without CAP_SYS_ADMIN.

See the top commit of
https://github.com/anbe42/dkms/commits/chroot-no-proc2

anbe42 · 2025-02-26T13:51:05Z

@akorn: I've slightly reorganized your branch and fixed a few whitespace errors while doing that: https://github.com/anbe42/dkms/commits/chroot-no-proc2

I would have liked to add some tests starting with umount /proc, but it looks like there are insufficient privileges/capabilities on the github runners to do that. Unless @scaronni knows a solution for that I'll merge without the extra tests.
At least I'll have a no-proc test in the Debian package CI.

scaronni · 2025-02-28T09:21:30Z

@anbe42 to be honest I don't know, I don't think we can avoid proc being mounted... so I would merge as is.

scaronni · 2025-03-01T16:06:42Z

@akorn please last quick rebase... @anbe42 I would like to do another release, can you please merge once rebased? Thanks.

The shift from the `< <(cmd)` idom to `<<<"$(cmd)"` which is needed to work without /proc being mounted has the unfortunate side effect that the exit status of `cmd` is ignored where it wasn't before. This can cause statements that previously always had valid input to be called on empty strings. This commit adds sanity checks to avoid processing the empty string where previously variables always contained something.

anbe42

Thanks again for your work!

anbe42 · 2025-03-02T10:16:53Z

dkms.in

@@ -827,6 +827,7 @@ get_module_verinfo(){
    vals=
    while read -ra vals; do
        [[ ${#vals[@]} -eq 0 ]] && continue
+        [[ ${vals[0]} ]] || continue


I have replaced that with a check for a non-empty array, not a non-empty first element.

anbe42 · 2025-03-02T10:18:13Z

Merged with some white-space errors fixed.

anbe42 · 2025-03-02T10:24:01Z

@anbe42 I would like to do another release, can you please merge once rebased? Thanks.
I'm currently rebasing my queued changes ... should have a new PR ready tonight

scaronni added the enhancement label Jan 29, 2025

scaronni requested a review from anbe42 February 4, 2025 08:51

akorn force-pushed the chroot-no-proc branch 2 times, most recently from 26afe79 to 57e045b Compare February 12, 2025 13:31

anbe42 reviewed Feb 13, 2025

View reviewed changes

dkms.in Outdated Show resolved Hide resolved

anbe42 requested changes Feb 13, 2025

View reviewed changes

akorn force-pushed the chroot-no-proc branch from 11af52a to 16e4d5a Compare February 14, 2025 15:08

anbe42 reviewed Feb 16, 2025

View reviewed changes

dkms.in Outdated Show resolved Hide resolved

akorn force-pushed the chroot-no-proc branch from 16e4d5a to aa27325 Compare February 23, 2025 09:02

akorn force-pushed the chroot-no-proc branch from aa27325 to 6e488b5 Compare February 23, 2025 09:22

akorn marked this pull request as draft February 23, 2025 09:46

akorn force-pushed the chroot-no-proc branch from 6e488b5 to 1265f10 Compare February 23, 2025 09:48

akorn marked this pull request as ready for review February 23, 2025 09:51

akorn requested a review from anbe42 February 23, 2025 13:06

anbe42 self-assigned this Feb 26, 2025

akorn added 2 commits March 1, 2025 18:15

Fix shellcheck warnings

7b084be

akorn force-pushed the chroot-no-proc branch from f2a29ed to 7b084be Compare March 1, 2025 17:20

anbe42 approved these changes Mar 2, 2025

View reviewed changes

anbe42 closed this Mar 2, 2025

Allow dkms to work in a chroot environment without /proc #485

Allow dkms to work in a chroot environment without /proc #485

Conversation

akorn commented Jan 28, 2025

scaronni commented Jan 29, 2025

akorn commented Jan 31, 2025

akorn commented Feb 4, 2025

anbe42 commented Feb 4, 2025

akorn commented Feb 4, 2025

akorn commented Feb 6, 2025

anbe42 commented Feb 7, 2025

scaronni commented Feb 10, 2025

akorn commented Feb 10, 2025 • edited Loading

akorn commented Feb 10, 2025

scaronni commented Feb 11, 2025

anbe42 commented Feb 11, 2025

scaronni commented Feb 11, 2025

scaronni commented Feb 12, 2025

anbe42 left a comment

Choose a reason for hiding this comment

akorn commented Feb 13, 2025

anbe42 commented Feb 13, 2025

akorn commented Feb 13, 2025

anbe42 commented Feb 13, 2025

anbe42 commented Feb 13, 2025

anbe42 left a comment

Choose a reason for hiding this comment

akorn commented Feb 17, 2025

scaronni commented Feb 22, 2025

akorn commented Feb 23, 2025

scaronni commented Feb 24, 2025

akorn commented Feb 24, 2025

scaronni commented Feb 24, 2025

anbe42 commented Feb 26, 2025

anbe42 commented Feb 26, 2025

scaronni commented Feb 28, 2025

scaronni commented Mar 1, 2025

anbe42 left a comment

Choose a reason for hiding this comment

anbe42 Mar 2, 2025

Choose a reason for hiding this comment

anbe42 commented Mar 2, 2025

anbe42 commented Mar 2, 2025

akorn commented Feb 10, 2025 •

edited

Loading