From ce6500893f24004a0d46dec4f671134eefed172b Mon Sep 17 00:00:00 2001 From: Junjie Mao Date: Wed, 16 Nov 2022 18:28:15 +0800 Subject: [PATCH] board_inspector: use executables found under system paths Using partial executable paths in the board inspector may cause unintended results when another executable has the same name and is also detectable in the search paths. Introduce a wrapper module (`external_tools`) which locates executables only under system paths such as /usr/bin and /usr/sbin and converts partial executable paths to absolute ones before executing them via the subprocess module. All invocations to `subprocess.run` or `subprocess.Popen` throughout the board inspector are replaced with `external_tools.run`, with the only exception being the invocation to the legacy board parser which already uses an absolute path to the current Python interpreter. Tracked-On: #8315 Signed-off-by: Junjie Mao --- .../board_inspector/board_inspector.py | 38 ++++------ .../board_inspector/cpuparser/platformbase.py | 4 +- .../extractors/10-processors.py | 1 - .../inspectorlib/external_tools.py | 75 +++++++++++++++++++ .../board_inspector/legacy/acpi.py | 15 +--- .../board_inspector/legacy/board_parser.py | 9 ++- .../board_inspector/legacy/clos.py | 6 +- .../board_inspector/legacy/dmar.py | 4 +- .../board_inspector/legacy/misc.py | 3 +- .../board_inspector/legacy/parser_lib.py | 24 +----- 10 files changed, 111 insertions(+), 68 deletions(-) create mode 100644 misc/config_tools/board_inspector/inspectorlib/external_tools.py diff --git a/misc/config_tools/board_inspector/board_inspector.py b/misc/config_tools/board_inspector/board_inspector.py index 645f1b375..51e143590 100755 --- a/misc/config_tools/board_inspector/board_inspector.py +++ b/misc/config_tools/board_inspector/board_inspector.py @@ -20,7 +20,7 @@ script_dir = os.path.dirname(os.path.realpath(__file__)) sys.path.append(os.path.join(script_dir)) from cpuparser import parse_cpuid, get_online_cpu_ids, get_offline_cpu_ids -from inspectorlib import validator +from inspectorlib import external_tools, validator class AddLLCCATAction(argparse.Action): CATInfo = namedtuple("CATInfo", ["capacity_mask_length", "clos_number", "has_CDP"]) @@ -38,37 +38,29 @@ class AddLLCCATAction(argparse.Action): def check_deps(): # Check that the required tools are installed on the system - BIN_LIST = ['cpuid', 'rdmsr', 'lspci', ' dmidecode', 'blkid', 'stty'] - cpuid_min_ver = 20170122 - had_error = False - for execute in BIN_LIST: - res = subprocess.Popen("which {}".format(execute), - shell=True, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, close_fds=True) + had_error = not external_tools.locate_tools(["cpuid", "rdmsr", "lspci", "dmidecode", "blkid", "stty", "modprobe"]) - line = res.stdout.readline().decode('ascii') - if not line: - logger.critical("'{}' cannot be found. Please install it and run the Board Inspector again.".format(execute)) + try: + cpuid_min_ver = 20170122 + res = external_tools.run("cpuid -v") + line = res.stdout.readline().decode("ascii") + version = line.split()[2] + if int(version) < cpuid_min_ver: + logger.critical("This tool requires CPUID version >= {}. Try updating and upgrading the OS" \ + "on this system and reruning the Board Inspector. If that fails, install a newer CPUID tool" \ + "from https://github.com/tycho/cpuid.".format(cpuid_min_ver)) had_error = True + except external_tools.ExecutableNotFound: + pass - if execute == 'cpuid': - res = subprocess.Popen("cpuid -v", - shell=True, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, close_fds=True) - line = res.stdout.readline().decode('ascii') - version = line.split()[2] - if int(version) < cpuid_min_ver: - logger.critical("This tool requires CPUID version >= {}. Try updating and upgrading the OS" \ - "on this system and reruning the Board Inspector. If that fails, install a newer CPUID tool" \ - "from https://github.com/tycho/cpuid.".format(cpuid_min_ver)) - had_error = True if had_error: sys.exit(1) # Try updating pci.ids for latest PCI device descriptions + external_tools.locate_tools(["update-pciids"]) try: logger.info("Updating pci.ids for latest PCI device descriptions.") - res = subprocess.Popen(["update-pciids", "-q"], stderr=subprocess.DEVNULL) + res = external_tools.run("update-pciids -q", stderr=subprocess.DEVNULL) if res.wait(timeout=40) != 0: logger.warning(f"Failed to invoke update-pciids. No functional impact is foreseen, but descriptions of PCI devices may be inaccurate.") except Exception as e: diff --git a/misc/config_tools/board_inspector/cpuparser/platformbase.py b/misc/config_tools/board_inspector/cpuparser/platformbase.py index 7acaa0661..a7cc5c152 100644 --- a/misc/config_tools/board_inspector/cpuparser/platformbase.py +++ b/misc/config_tools/board_inspector/cpuparser/platformbase.py @@ -7,7 +7,6 @@ from __future__ import print_function import sys -import subprocess # nosec import re import functools import inspect @@ -15,6 +14,7 @@ import operator import textwrap import logging from collections import namedtuple +from inspectorlib import external_tools _wrapper = textwrap.TextWrapper(width=78, initial_indent=' ', subsequent_indent=' ') regex_hex = "0x[0-9a-f]+" @@ -26,7 +26,7 @@ class cpuid_result(namedtuple('cpuid_result', ['eax', 'ebx', 'ecx', 'edx'])): return "cpuid_result(eax={eax:#010x}, ebx={ebx:#010x}, ecx={ecx:#010x}, edx={edx:#010x})".format(**self._asdict()) def cpuid(cpu_id, leaf, subleaf): - result = subprocess.run(["cpuid", "-l", str(leaf), "-s", str(subleaf), "-r"], stdout=subprocess.PIPE, check=True) + result = external_tools.run(["cpuid", "-l", str(leaf), "-s", str(subleaf), "-r"], check=True) stdout = result.stdout.decode("ascii").replace("\n", "") regex = re.compile(f"CPU {cpu_id}:[^:]*: eax=({regex_hex}) ebx=({regex_hex}) ecx=({regex_hex}) edx=({regex_hex})") m = regex.search(stdout) diff --git a/misc/config_tools/board_inspector/extractors/10-processors.py b/misc/config_tools/board_inspector/extractors/10-processors.py index c8e80750a..ac73a2200 100644 --- a/misc/config_tools/board_inspector/extractors/10-processors.py +++ b/misc/config_tools/board_inspector/extractors/10-processors.py @@ -4,7 +4,6 @@ # import logging -import subprocess import lxml.etree import re diff --git a/misc/config_tools/board_inspector/inspectorlib/external_tools.py b/misc/config_tools/board_inspector/inspectorlib/external_tools.py new file mode 100644 index 000000000..d8951a4cc --- /dev/null +++ b/misc/config_tools/board_inspector/inspectorlib/external_tools.py @@ -0,0 +1,75 @@ +# Copyright (C) 2022 Intel Corporation. +# +# SPDX-License-Identifier: BSD-3-Clause +# + +import os +import logging +import subprocess # nosec + +available_tools = {} + +class ExecutableNotFound(ValueError): + pass + +def run(command, **kwargs): + """Run the given command which can be either a string or a list.""" + try: + if isinstance(command, list): + full_path = available_tools[command[0]] + kwargs.setdefault("capture_output", True) + return subprocess.run([full_path] + command[1:], **kwargs) + elif isinstance(command, str): + parts = command.split(" ", maxsplit=1) + full_path = available_tools[parts[0]] + kwargs["shell"] = True + kwargs.setdefault("stdout", subprocess.PIPE) + kwargs.setdefault("stderr", subprocess.PIPE) + kwargs.setdefault("close_fds", True) + cmd = f"{full_path} {parts[1]}" if len(parts) == 2 else full_path + return subprocess.Popen(cmd, **kwargs) + except KeyError: + raise ExecutableNotFound + + assert False, f"A command is expected either a list or a string: {command}" + +def detect_tool(name): + """Detect the full path of a system tool.""" + system_paths = [ + "/usr/bin/", + "/usr/sbin/", + "/usr/local/bin/", + "/usr/local/sbin/", + ] + + # Look for `which` first. + for path in system_paths: + candidate = os.path.join(path, "which") + if os.path.exists(candidate): + available_tools["which"] = candidate + break + + try: + result = run(["which", "-a", name]) + for path in result.stdout.decode().strip().split("\n"): + if any(map(lambda x: path.startswith(x), system_paths)): + logging.debug(f"Use {name} found at {path}.") + return path + except ExecutableNotFound: + pass + + logging.critical(f"'{name}' cannot be found. Please install it and run again.") + return None + +def locate_tools(tool_list): + """Find a list of tools under common system executable paths. Return True if and only if all tools are found.""" + had_error = False + + for tool in tool_list: + full_path = detect_tool(tool) + if full_path != None: + available_tools[tool] = full_path + else: + had_error = True + + return not had_error diff --git a/misc/config_tools/board_inspector/legacy/acpi.py b/misc/config_tools/board_inspector/legacy/acpi.py index 3633645c4..4d87f8e63 100644 --- a/misc/config_tools/board_inspector/legacy/acpi.py +++ b/misc/config_tools/board_inspector/legacy/acpi.py @@ -5,12 +5,12 @@ import os import sys -import subprocess # nosec import shutil from collections import defaultdict import dmar import parser_lib import logging +from inspectorlib import external_tools SYS_PATH = ['/proc/cpuinfo', '/sys/firmware/acpi/tables/', '/sys/devices/system/cpu/'] @@ -546,14 +546,11 @@ def store_px_data(sysnode, config): p_cnt = 0 for freq in freqs.split(): if boost != 0 and i == 0: - try: - subprocess.check_call('/usr/sbin/rdmsr 0x1ad', shell=True, stdout=subprocess.PIPE) - except subprocess.CalledProcessError: + res = external_tools.run(['rdmsr', '0x1ad']) + if res.returncode != 0: logging.debug("MSR 0x1ad not support in this platform!") return - res = subprocess.Popen('/usr/sbin/rdmsr 0x1ad', shell=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True) result = res.stdout.readline().strip() #max_ratio_cpu = result[-2:] ctl_state = int(result[-2:], 16) << 8 @@ -600,11 +597,7 @@ def read_tpm_data(config): :param config: file pointer that opened for writing board config information :return: ''' - try: - acpi_table_output = subprocess.check_output('ls -l /sys/firmware/acpi/tables/'.split()).decode('utf8') - except: - acpi_table_output = '' - if 'TPM2' in acpi_table_output: + if os.path.exists('/sys/firmware/acpi/tables/TPM2'): print("\tTPM2", file=config) else: print("\t/* no TPM device */", file=config) diff --git a/misc/config_tools/board_inspector/legacy/board_parser.py b/misc/config_tools/board_inspector/legacy/board_parser.py index ec2ce050c..b896b219b 100755 --- a/misc/config_tools/board_inspector/legacy/board_parser.py +++ b/misc/config_tools/board_inspector/legacy/board_parser.py @@ -7,7 +7,6 @@ import os import sys import shutil import argparse -import subprocess # nosec import pci_dev import dmi import acpi @@ -15,6 +14,7 @@ import clos import misc import parser_lib import logging +from inspectorlib import external_tools OUTPUT = "./out/" PY_CACHE = "__pycache__" @@ -52,12 +52,13 @@ def check_env(): if os.path.exists(PY_CACHE): shutil.rmtree(PY_CACHE) + if not external_tools.locate_tools(['cpuid', 'rdmsr', 'lspci', 'dmidecode', 'blkid', 'stty', 'modprobe']): + sys.exit(1) + # check cpu msr file cpu_dirs = "/dev/cpu" if not check_msr_files(cpu_dirs): - res = subprocess.Popen("modprobe msr", - shell=True, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, close_fds=True) + res = external_tools.run("modprobe msr") err_msg = res.stderr.readline().decode('ascii') if err_msg: logging.critical("{}".format(err_msg)) diff --git a/misc/config_tools/board_inspector/legacy/clos.py b/misc/config_tools/board_inspector/legacy/clos.py index 61ccb9383..64eef3626 100644 --- a/misc/config_tools/board_inspector/legacy/clos.py +++ b/misc/config_tools/board_inspector/legacy/clos.py @@ -3,8 +3,8 @@ # SPDX-License-Identifier: BSD-3-Clause # -import parser_lib import logging +from inspectorlib import external_tools RDT_TYPE = { "L2":4, @@ -18,7 +18,7 @@ def dump_cpuid_reg(cmd, reg): :param cmd: command what can be executed in shell :param reg: register name """ - res = parser_lib.cmd_execute(cmd) + res = external_tools.run(cmd) if reg == "eax": idx = 2 if reg == "ebx": @@ -27,7 +27,7 @@ def dump_cpuid_reg(cmd, reg): idx = 5 while True: - line = parser_lib.decode_stdout(res) + line = res.stdout.readline().decode('ascii') if not line: break diff --git a/misc/config_tools/board_inspector/legacy/dmar.py b/misc/config_tools/board_inspector/legacy/dmar.py index 6b0796d16..f4c47b4a9 100644 --- a/misc/config_tools/board_inspector/legacy/dmar.py +++ b/misc/config_tools/board_inspector/legacy/dmar.py @@ -5,7 +5,7 @@ import sys import ctypes -import parser_lib +from inspectorlib import external_tools ACPI_DMAR_TYPE = { 'ACPI_DMAR_TYPE_HARDWARE_UNIT':0, @@ -193,7 +193,7 @@ def get_secondary_bus(dmar_tbl, tmp_dev, tmp_fun): secondary_bus_str = '' found_pci_bdf = False pci_bridge_header_type = False - res = parser_lib.cmd_execute(cmd) + res = external_tools.run(cmd) while True: line = res.stdout.readline().decode("ascii") diff --git a/misc/config_tools/board_inspector/legacy/misc.py b/misc/config_tools/board_inspector/legacy/misc.py index 47a7d40ac..fcc811737 100644 --- a/misc/config_tools/board_inspector/legacy/misc.py +++ b/misc/config_tools/board_inspector/legacy/misc.py @@ -4,6 +4,7 @@ # import parser_lib, os +from inspectorlib import external_tools from extractors.helpers import get_bdf_from_realpath MEM_PATH = ['/proc/iomem', '/proc/meminfo'] @@ -35,7 +36,7 @@ def detected_ttys(): tty_used_list = [] for s_inc in range(ttys_cnt): cmd = 'stty -F /dev/ttyS{}'.format(s_inc) - res = parser_lib.cmd_execute('{}'.format(cmd)) + res = external_tools.run('{}'.format(cmd)) while True: line = res.stdout.readline().decode('ascii') diff --git a/misc/config_tools/board_inspector/legacy/parser_lib.py b/misc/config_tools/board_inspector/legacy/parser_lib.py index a2433ab86..3794fb399 100644 --- a/misc/config_tools/board_inspector/legacy/parser_lib.py +++ b/misc/config_tools/board_inspector/legacy/parser_lib.py @@ -4,7 +4,7 @@ # import os -import subprocess # nosec +from inspectorlib import external_tools BIOS_INFO_KEY = ['BIOS Information', 'Vendor:', 'Version:', 'Release Date:', 'BIOS Revision:'] @@ -45,14 +45,6 @@ def print_red(msg, err=False): print("\033[1;31m{0}\033[0m".format(msg)) -def decode_stdout(resource): - """Decode the information and return one line of the decoded information - :param resource: it contains information produced by subprocess.Popen method - """ - line = resource.stdout.readline().decode('ascii') - return line - - def handle_hw_info(line, hw_info): """Handle the hardware information :param line: one line of information which had decoded to 'ASCII' @@ -80,16 +72,6 @@ def handle_pci_dev(line): return False -def cmd_execute(cmd): - """Excute cmd and retrun raw information - :param cmd: command what can be executed in shell - """ - res = subprocess.Popen(cmd, shell=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True) - - return res - - def handle_block_dev(line): """Handle if it match root device information pattern :param line: one line of information which had decoded to 'ASCII' @@ -122,7 +104,7 @@ def dump_execute(cmd, desc, config): print("\t\t".format(desc), file=config) return - res = cmd_execute(cmd) + res = external_tools.run(cmd) while True: line = res.stdout.readline().decode('ascii') line = line.replace("&", "&").replace('"', """) \ @@ -160,7 +142,7 @@ def dump_execute(cmd, desc, config): def get_output_lines(cmd): res_lines = [] - res = cmd_execute(cmd) + res = external_tools.run(cmd) while True: line = res.stdout.readline().decode('ascii') if not line: