[Pkg-dkms-maint] Bug#890644: dkms: DKMS confuses module versions

[Nikolai Kondrashov]

Package: dkms
Version: 2.3-3
Severity: important
Tags: patch

Dear Maintainer,

While testing installation of digimend-dkms .deb package (built from master of
https://github.com/DIGImend/digimend-kernel-drivers) it appeared that one of
the modules (hid-uclogic.ko) is not installed, as DKMS considers it "not newer
than what is already found in kernel". However, none of the modules being
installed have a version in kernel (on purpose), so all the modules should
have installed correctly.

The issue turned out to be a misuse of Bash "unset" command. It was
(incorrectly) used to clear the variable containing results of version
extraction, which led to an in-kernel module getting version information of
the previous module installed by DKMS (which would be the same for all modules
being installed).

See details in upstream pull request: https://github.com/dell/dkms/pull/47

-- System Information:
Debian Release: buster/sid
  APT prefers testing
  APT policy: (700, 'testing'), (600, 'unstable'), (550, 'stable'), (500, 'testing-debug')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.14.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages dkms depends on:
ii  build-essential  12.4
ii  coreutils        8.28-1
ii  dpkg-dev         1.19.0.5
ii  gcc              4:7.2.0-1d1
ii  kmod             25-1
ii  make             4.1-9.1
ii  patch            2.7.5-1+b2

Versions of packages dkms recommends:
ii  fakeroot             1.22-2
ii  linux-headers-amd64  4.14+89
ii  lsb-release          9.20170808
ii  sudo                 1.8.21p2-3

Versions of packages dkms suggests:
ii  menu            2.1.47+b1
pn  python3-apport  <none>

-- no debconf information
-------------- next part --------------
>From 78f5466b4fc5b763eca98568ce83ecc9a1249cf5 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <spbnick at gmail.com>
Date: Mon, 12 Feb 2018 20:20:15 +0200
Subject: [PATCH] Do not unset local array variable in Bash func

Do not unset local array variable to clear it in "get_module_verinfo"
function in "dkms" script. Assign empty array instead.

When a local variable is "unset" in Bash, it is completely removed, and
its scope is lost. If a new value is assigned to the same name
afterwards (and there was no local variable with the same name up the
stack), a new variable is created, this time with the global scope.

If then a local variable is created with the same name again, it hides
the global one, and if that is "unset" again, the global one becomes
visible again.

This is what happens with the "check_version_sanity" and
"get_module_verinfo" functions in "dkms" script. It works OK, if both
kernel and installed module have version info, but when there is more
than one module to process, and a second or later in-kernel module
("$kernels_module") lacks any version info in the kernel, the
"check_version_sanity" function uses the last built module's version
info instead.

I.e. this is how "check_version_sanity" would work for a module with
version information in both kernel and dkms build:

    # Unset local "res", assign version info to global "res"
    get_module_verinfo $kernels_module; kernels_info=("${res[@]}")
    # Unset global "res", assign version info to global "res"
    get_module_verinfo $dkms_module; dkms_info=("${res[@]}")

This is how "check_version_sanity" would work for a module without
version information in the kernel, but with version information in the
module being installed

    # Unset local "res", do not assign anything
    get_module_verinfo $kernels_module; kernels_info=("${res[@]}")
    # Unset global "res", if any, assign version info to global "res"
    get_module_verinfo $dkms_module; dkms_info=("${res[@]}")

And for a second invocation of "check_version_sanity" for a similar
module:

    # Unset local "res", do not assign anything
    # This uses the value of global "res" assigned last time
    get_module_verinfo $kernels_module; kernels_info=("${res[@]}")
    # Unset global "res", assign result to global "res"
    get_module_verinfo $dkms_module; dkms_info=("${res[@]}")
---
 dkms | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dkms b/dkms
index 5a9bb29..b5b792d 100644
--- a/dkms
+++ b/dkms
@@ -742,7 +742,7 @@ read_conf()
 
 # Little helper function for parsing the output of modinfo.
 get_module_verinfo(){
-    unset res
+    res=()
     local vals=
     while read -a vals; do
     case ${vals[0]} in
-- 
2.16.1