diff mbox series

[autotest-client-tests] UBUNTU: SAUCE: ubuntu_boot: add exception for modules on DGX

Message ID 20220427083849.258299-1-po-hsu.lin@canonical.com
State New
Headers show
Series [autotest-client-tests] UBUNTU: SAUCE: ubuntu_boot: add exception for modules on DGX | expand

Commit Message

Po-Hsu Lin April 27, 2022, 8:38 a.m. UTC
Server HWE team uses a preseed script to install extra modules on DGX
systems, this is mimicking user-cases but these out-of-tree modules
and unsigned modules will taints the kernel:
    mlx_compat: loading out-of-tree module taints kernel.
    mlx_compat: module verification failed: signature and/or required
                key missing - tainting kernel

Thus cause the kernel_tainted test failing like:
    Kernel taint value is 12288
    Taint bit value: 12 (externally-built ('out-of-tree') module was loaded)
    *   Modules not in-tree:
         mlx5_ib
         ib_uverbs
         ib_core
         mlx5_core
         mlxdevm
         auxiliary
         mlxfw
         mlx_compat
    Taint bit value: 13 (unsigned module was loaded)

Add these modules to an exclusion list so that we don't need to hint
this failure for every cycle.

Note that this failure is only existing on Bionic / Focal with DGX
systems. On Impish / Jammy despite these modules were loaded as well
but they are not tainting the kernel. To incorporate this, limit the
exception check on B/F with DGX systems by looking for "DGX" string
in the product name.

Patch tested with DGX node "exotic-skunk" on B/F, passed as expected.

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 ubuntu_boot/kernel_taint_test.py | 113 +++++++++++++++++++++++++++----
 1 file changed, 100 insertions(+), 13 deletions(-)

Comments

Francis Ginther April 28, 2022, 9:59 p.m. UTC | #1
Acked-by: Francis Ginther <francis.ginther@canonical.com>

See inline comment.

On Wed, Apr 27, 2022 at 04:38:49PM +0800, Po-Hsu Lin wrote:
> Server HWE team uses a preseed script to install extra modules on DGX
> systems, this is mimicking user-cases but these out-of-tree modules
> and unsigned modules will taints the kernel:
>     mlx_compat: loading out-of-tree module taints kernel.
>     mlx_compat: module verification failed: signature and/or required
>                 key missing - tainting kernel
> 
> Thus cause the kernel_tainted test failing like:
>     Kernel taint value is 12288
>     Taint bit value: 12 (externally-built ('out-of-tree') module was loaded)
>     *   Modules not in-tree:
>          mlx5_ib
>          ib_uverbs
>          ib_core
>          mlx5_core
>          mlxdevm
>          auxiliary
>          mlxfw
>          mlx_compat
>     Taint bit value: 13 (unsigned module was loaded)
> 
> Add these modules to an exclusion list so that we don't need to hint
> this failure for every cycle.
> 
> Note that this failure is only existing on Bionic / Focal with DGX
> systems. On Impish / Jammy despite these modules were loaded as well
> but they are not tainting the kernel. To incorporate this, limit the
> exception check on B/F with DGX systems by looking for "DGX" string
> in the product name.
> 
> Patch tested with DGX node "exotic-skunk" on B/F, passed as expected.
> 
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  ubuntu_boot/kernel_taint_test.py | 113 +++++++++++++++++++++++++++----
>  1 file changed, 100 insertions(+), 13 deletions(-)
> 
> diff --git a/ubuntu_boot/kernel_taint_test.py b/ubuntu_boot/kernel_taint_test.py
> index 0e8a7eaa..277be563 100755
> --- a/ubuntu_boot/kernel_taint_test.py
> +++ b/ubuntu_boot/kernel_taint_test.py
> @@ -28,6 +28,7 @@ returns a value of 1. The script also returns a value of 1 if
>  """
>  
>  
> +import platform
>  import sys
>  import shlex
>  from argparse import ArgumentParser
> @@ -60,23 +61,81 @@ def get_modules():
>      return modules
>  
>  
> -def print_out_of_tree_modules(modules):
> -    print("*   Modules not in-tree:")
> +def process_out_of_tree_modules(modules):
> +    mod_list = []
> +    modules = remove_ignored_modules(modules)
>      for mod in modules:
>          cmd = 'modinfo -F intree %s' % mod
>          if not check_output(shlex.split(cmd),
>                              universal_newlines=True):
> -            print("     %s" % mod)
> +            mod_list.append(mod)
> +    return(mod_list)
>  
>  
> -def print_GPL_incompatible_modules(modules):
> -    print("*   Modules with GPL Incompatible Licenses:")
> +def process_GPL_incompatible_modules(modules):
> +    mod_list = []
> +    modules = remove_ignored_modules(modules)
>      for mod in modules:
>          cmd = 'modinfo -F license %s' % mod
>          license = check_output(shlex.split(cmd),
>                                 universal_newlines=True).strip()
>          if "GPL" not in license and "MIT" not in license:
> -            print("     %s: %s" % (mod, license))
> +            mod_list.append((mod, license))
> +    return(mod_list)
> +
> +
> +def process_unsigned_modules(modules):
> +    mod_list = []
> +    modules = remove_ignored_modules(modules)
> +    for mod in modules:
> +        fn = '/sys/module/{}/taint'.format(mod)
> +        with open(fn, 'r') as f:
> +            status = f.read()
> +            if 'E' in status:
> +                mod_list.append(mod)
> +    return(mod_list)
> +
> +
> +def remove_ignored_modules(modules):
> +    # Remove modules we know will fail, but accept
> +    dgx_modules = {'focal': ['mlx5_ib',
> +                             'ib_uverbs',
> +                             'ib_core',
> +                             'mlx5_core',
> +                             'mlxdevm',
> +                             'auxiliary',
> +                             'mlxfw',
> +                             'mlx_compat'],
> +                   'bionic':['ib_iser',
> +                             'rdma_cm',
> +                             'iw_cm',
> +                             'ib_cm',
> +                             'mlx5_ib',
> +                             'ib_uverbs',
> +                             'ib_core',
> +                             'mlx5_core',
> +                             'mlxfw',
> +                             'mdev',
> +                             'mlx_compat']}
> +    try:
> +        series = platform.dist()[2]
> +    except AttributeError:
> +        import distro
> +        series = distro.codename()
> +    try:
> +        with open('/sys/class/dmi/id/product_name', 'r') as f:
> +            product_name = f.read().strip()
> +    except FileNotFoundError:
> +        product_name = ''
> +
> +    if series in ['focal', 'bionic'] and 'DGX' in product_name:
> +        for ignore_mod in dgx_modules[series]:
> +            try:
> +                print('Exception made in test script: {}'.format(ignore_mod))
> +                modules.remove(ignore_mod)
> +            except ValueError:
> +                pass
> +    return(modules)
>  
>  
>  def main():
> @@ -114,16 +173,44 @@ def main():
>              modules = get_modules()
>              print("Taint bit value: {} ({})".format(i, taint_meanings[i]))
>              if i == 0:  # List GPL incompatible modules and licenses
> -                print_GPL_incompatible_modules(modules)
> -            if i == 12:  # List out-of-tree modules
> -                print_out_of_tree_modules(modules)
> -            if i == 11:
> +                proprietary_modules = process_GPL_incompatible_modules(modules)
> +                if proprietary_modules:
> +                    print("*   Modules with GPL Incompatible Licenses:")
> +                    for mod in proprietary_modules:
> +                        print("     %s: %s" % (mod[0], mod[1]))
> +                    count += 1
> +                else:
> +                    print("*   Proprietary modules found, "
> +                          "but they are expected and OK")
> +            elif i == 11:
>                  print("*   Firmware workarounds are expected and OK")
> -                continue
> -            count += 1
> +            elif i == 12:  # List out-of-tree modules
> +                out_of_tree_modules = process_out_of_tree_modules(modules)
> +                if len(out_of_tree_modules) > 0:
> +                    print("*   Modules not in-tree:")
> +                    for mod in out_of_tree_modules:
> +                        print("     %s" % mod)
> +                    count += 1
> +                else:
> +                    print("*   Out of Tree modules found, "
> +                          "but they are expected and OK")
> +            elif i == 13:
> +                unsigned_modules = process_unsigned_modules(modules)
> +                if len(unsigned_modules) > 0:
> +                    print("*   Module not signed / failed with verification")
> +                    for mod in unsigned_modules:
> +                        print("     %s" % mod)
> +                    count += 1
> +                else:
> +                    print("*   Unsigned modules found, "
> +                          "but they are expected and OK")
> +            else:
> +                print("Taint bit value: {} ({})".format(i, taint_meanings[i]))

This print is unnecessary. It matches the same print statement before
the start of the if block and results in the following for taint values
without an explicit handler.

$ ./kernel_taint_test.py 
Kernel taint value is 9216
Taint bit value: 10 (staging driver was loaded)
Taint bit value: 10 (staging driver was loaded)
Taint bit value: 13 (unsigned module was loaded)
*   Module not signed / failed with verification
    bcm2835_codec
    bcm2835_isp
    ...


> +                count += 1
> +
>  
>      if count == 0:
> -        # else case below contains expected issue in case 0 / 11 / 12
> +        # else case below contains expected issue in case 0 / 11 / 12 / 13
>          if not taints:
>              print("No kernel taints detected.")
>          return 0
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Po-Hsu Lin April 29, 2022, 4:30 a.m. UTC | #2
Hey Francis,
Thanks for catching this extra print issue, yes this should not be
added and I have this print removed when applying this patch.

Applied and pushed.

Thanks
Sam
diff mbox series

Patch

diff --git a/ubuntu_boot/kernel_taint_test.py b/ubuntu_boot/kernel_taint_test.py
index 0e8a7eaa..277be563 100755
--- a/ubuntu_boot/kernel_taint_test.py
+++ b/ubuntu_boot/kernel_taint_test.py
@@ -28,6 +28,7 @@  returns a value of 1. The script also returns a value of 1 if
 """
 
 
+import platform
 import sys
 import shlex
 from argparse import ArgumentParser
@@ -60,23 +61,81 @@  def get_modules():
     return modules
 
 
-def print_out_of_tree_modules(modules):
-    print("*   Modules not in-tree:")
+def process_out_of_tree_modules(modules):
+    mod_list = []
+    modules = remove_ignored_modules(modules)
     for mod in modules:
         cmd = 'modinfo -F intree %s' % mod
         if not check_output(shlex.split(cmd),
                             universal_newlines=True):
-            print("     %s" % mod)
+            mod_list.append(mod)
+    return(mod_list)
 
 
-def print_GPL_incompatible_modules(modules):
-    print("*   Modules with GPL Incompatible Licenses:")
+def process_GPL_incompatible_modules(modules):
+    mod_list = []
+    modules = remove_ignored_modules(modules)
     for mod in modules:
         cmd = 'modinfo -F license %s' % mod
         license = check_output(shlex.split(cmd),
                                universal_newlines=True).strip()
         if "GPL" not in license and "MIT" not in license:
-            print("     %s: %s" % (mod, license))
+            mod_list.append((mod, license))
+    return(mod_list)
+
+
+def process_unsigned_modules(modules):
+    mod_list = []
+    modules = remove_ignored_modules(modules)
+    for mod in modules:
+        fn = '/sys/module/{}/taint'.format(mod)
+        with open(fn, 'r') as f:
+            status = f.read()
+            if 'E' in status:
+                mod_list.append(mod)
+    return(mod_list)
+
+
+def remove_ignored_modules(modules):
+    # Remove modules we know will fail, but accept
+    dgx_modules = {'focal': ['mlx5_ib',
+                             'ib_uverbs',
+                             'ib_core',
+                             'mlx5_core',
+                             'mlxdevm',
+                             'auxiliary',
+                             'mlxfw',
+                             'mlx_compat'],
+                   'bionic':['ib_iser',
+                             'rdma_cm',
+                             'iw_cm',
+                             'ib_cm',
+                             'mlx5_ib',
+                             'ib_uverbs',
+                             'ib_core',
+                             'mlx5_core',
+                             'mlxfw',
+                             'mdev',
+                             'mlx_compat']}
+    try:
+        series = platform.dist()[2]
+    except AttributeError:
+        import distro
+        series = distro.codename()
+    try:
+        with open('/sys/class/dmi/id/product_name', 'r') as f:
+            product_name = f.read().strip()
+    except FileNotFoundError:
+        product_name = ''
+
+    if series in ['focal', 'bionic'] and 'DGX' in product_name:
+        for ignore_mod in dgx_modules[series]:
+            try:
+                print('Exception made in test script: {}'.format(ignore_mod))
+                modules.remove(ignore_mod)
+            except ValueError:
+                pass
+    return(modules)
 
 
 def main():
@@ -114,16 +173,44 @@  def main():
             modules = get_modules()
             print("Taint bit value: {} ({})".format(i, taint_meanings[i]))
             if i == 0:  # List GPL incompatible modules and licenses
-                print_GPL_incompatible_modules(modules)
-            if i == 12:  # List out-of-tree modules
-                print_out_of_tree_modules(modules)
-            if i == 11:
+                proprietary_modules = process_GPL_incompatible_modules(modules)
+                if proprietary_modules:
+                    print("*   Modules with GPL Incompatible Licenses:")
+                    for mod in proprietary_modules:
+                        print("     %s: %s" % (mod[0], mod[1]))
+                    count += 1
+                else:
+                    print("*   Proprietary modules found, "
+                          "but they are expected and OK")
+            elif i == 11:
                 print("*   Firmware workarounds are expected and OK")
-                continue
-            count += 1
+            elif i == 12:  # List out-of-tree modules
+                out_of_tree_modules = process_out_of_tree_modules(modules)
+                if len(out_of_tree_modules) > 0:
+                    print("*   Modules not in-tree:")
+                    for mod in out_of_tree_modules:
+                        print("     %s" % mod)
+                    count += 1
+                else:
+                    print("*   Out of Tree modules found, "
+                          "but they are expected and OK")
+            elif i == 13:
+                unsigned_modules = process_unsigned_modules(modules)
+                if len(unsigned_modules) > 0:
+                    print("*   Module not signed / failed with verification")
+                    for mod in unsigned_modules:
+                        print("     %s" % mod)
+                    count += 1
+                else:
+                    print("*   Unsigned modules found, "
+                          "but they are expected and OK")
+            else:
+                print("Taint bit value: {} ({})".format(i, taint_meanings[i]))
+                count += 1
+
 
     if count == 0:
-        # else case below contains expected issue in case 0 / 11 / 12
+        # else case below contains expected issue in case 0 / 11 / 12 / 13
         if not taints:
             print("No kernel taints detected.")
         return 0