Skip to content
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

Do not unset local array variable in Bash func #47

Merged
merged 1 commit into from
Mar 6, 2018

Conversation

spbnick
Copy link
Contributor

@spbnick spbnick commented Feb 12, 2018

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[@]}")

This was hit while testing installation of digimend-dkms-7 .deb package on
Debian Stable with DKMS v2.2.1.0. The installed kernel's module didn't have
version info, the modules being installed did, and the second module failed to
update, as DKMS thought in-kernel module version was the same (being taken
from the previous module being installed, which had the same version).

Sample output:

nick@debian-stable:~$ sudo dpkg -i digimend-dkms_7_all.deb 
Selecting previously unselected package digimend-dkms.
(Reading database ... 160560 files and directories currently installed.)
Preparing to unpack digimend-dkms_7_all.deb ...
Unpacking digimend-dkms (7) ...
Setting up digimend-dkms (7) ...
Loading new digimend-7 DKMS files...
Building for 4.9.0-5-amd64
Building initial module for 4.9.0-5-amd64
Done.

hid-kye:
Running module version sanity check.
 - Original module
 - Installation
   - Installing to /lib/modules/4.9.0-5-amd64/updates/dkms/

hid-uclogic.ko:
Running module version sanity check.
Error! Module version 7 for hid-uclogic.ko
is not newer than what is already found in kernel 4.9.0-5-amd64 (7).
You may override by specifying --force.

hid-polostar.ko:
Running module version sanity check.
 - Original module
 - Installation
   - Installing to /lib/modules/4.9.0-5-amd64/updates/dkms/

hid-viewsonic.ko:
Running module version sanity check.
 - Original module
 - Installation
   - Installing to /lib/modules/4.9.0-5-amd64/updates/dkms/

depmod...

DKMS: install completed.

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[@]}")
@spbnick
Copy link
Contributor Author

spbnick commented Feb 12, 2018

Here's an explanation from Bash developers: https://lists.gnu.org/archive/html/bug-bash/2018-02/msg00072.html

@scaronni scaronni merged commit b1b9033 into dell:master Mar 6, 2018
@spbnick
Copy link
Contributor Author

spbnick commented Mar 6, 2018

Thanks, Simone!

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

Successfully merging this pull request may close these issues.

None yet

2 participants