Re: [PATCH 10/10] efi/libstub: Check return value of efi_parse_options

[kbuild test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 11330 bytes --]

Hi Arvind,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on efi/next]
[also build test WARNING on next-20200429]
[cannot apply to v5.7-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Arvind-Sankar/efi-some-cleanups-refactoring-for-efi-next/20200430-051025
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: arm-defconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1ccde533425a4ba9d379510206ad680ff9702129)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/firmware/efi/libstub/efi-stub.c:220:7: warning: variable 'si' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
                   if (status != EFI_SUCCESS) {
                       ^~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:339:19: note: uninitialized use occurs here
           free_screen_info(si);
                            ^~
   drivers/firmware/efi/libstub/efi-stub.c:220:3: note: remove the 'if' if its condition is always false
                   if (status != EFI_SUCCESS) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:212:7: warning: variable 'si' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
                   if (status != EFI_SUCCESS) {
                       ^~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:339:19: note: uninitialized use occurs here
           free_screen_info(si);
                            ^~
   drivers/firmware/efi/libstub/efi-stub.c:212:3: note: remove the 'if' if its condition is always false
                   if (status != EFI_SUCCESS) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:161:24: note: initialize the variable 'si' to silence this warning
           struct screen_info *si;
                                 ^
                                  = NULL
   2 warnings generated.

vim +220 drivers/firmware/efi/libstub/efi-stub.c

   119	
   120	/*
   121	 * This function handles the architcture specific differences between arm and
   122	 * arm64 regarding where the kernel image must be loaded and any memory that
   123	 * must be reserved. On failure it is required to free all
   124	 * all allocations it has made.
   125	 */
   126	efi_status_t handle_kernel_image(unsigned long *image_addr,
   127					 unsigned long *image_size,
   128					 unsigned long *reserve_addr,
   129					 unsigned long *reserve_size,
   130					 unsigned long dram_base,
   131					 efi_loaded_image_t *image);
   132	
   133	asmlinkage void __noreturn efi_enter_kernel(unsigned long entrypoint,
   134						    unsigned long fdt_addr,
   135						    unsigned long fdt_size);
   136	
   137	/*
   138	 * EFI entry point for the arm/arm64 EFI stubs.  This is the entrypoint
   139	 * that is described in the PE/COFF header.  Most of the code is the same
   140	 * for both archictectures, with the arch-specific code provided in the
   141	 * handle_kernel_image() function.
   142	 */
   143	efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
   144	{
   145		efi_loaded_image_t *image;
   146		efi_status_t status;
   147		unsigned long image_addr;
   148		unsigned long image_size = 0;
   149		unsigned long dram_base;
   150		/* addr/point and size pairs for memory management*/
   151		unsigned long initrd_addr = 0;
   152		unsigned long initrd_size = 0;
   153		unsigned long fdt_addr = 0;  /* Original DTB */
   154		unsigned long fdt_size = 0;
   155		char *cmdline_ptr = NULL;
   156		int cmdline_size = 0;
   157		efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID;
   158		unsigned long reserve_addr = 0;
   159		unsigned long reserve_size = 0;
   160		enum efi_secureboot_mode secure_boot;
   161		struct screen_info *si;
   162		efi_properties_table_t *prop_tbl;
   163		unsigned long max_addr;
   164	
   165		efi_system_table = sys_table_arg;
   166	
   167		/* Check if we were booted by the EFI firmware */
   168		if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) {
   169			status = EFI_INVALID_PARAMETER;
   170			goto fail;
   171		}
   172	
   173		status = check_platform_features();
   174		if (status != EFI_SUCCESS)
   175			goto fail;
   176	
   177		/*
   178		 * Get a handle to the loaded image protocol.  This is used to get
   179		 * information about the running image, such as size and the command
   180		 * line.
   181		 */
   182		status = efi_system_table->boottime->handle_protocol(handle,
   183						&loaded_image_proto, (void *)&image);
   184		if (status != EFI_SUCCESS) {
   185			pr_efi_err("Failed to get loaded image protocol\n");
   186			goto fail;
   187		}
   188	
   189		dram_base = get_dram_base();
   190		if (dram_base == EFI_ERROR) {
   191			pr_efi_err("Failed to find DRAM base\n");
   192			status = EFI_LOAD_ERROR;
   193			goto fail;
   194		}
   195	
   196		/*
   197		 * Get the command line from EFI, using the LOADED_IMAGE
   198		 * protocol. We are going to copy the command line into the
   199		 * device tree, so this can be allocated anywhere.
   200		 */
   201		cmdline_ptr = efi_convert_cmdline(image, &cmdline_size, ULONG_MAX);
   202		if (!cmdline_ptr) {
   203			pr_efi_err("getting command line via LOADED_IMAGE_PROTOCOL\n");
   204			status = EFI_OUT_OF_RESOURCES;
   205			goto fail;
   206		}
   207	
   208		if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
   209		    IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
   210		    cmdline_size == 0) {
   211			status = efi_parse_options(CONFIG_CMDLINE);
   212			if (status != EFI_SUCCESS) {
   213				pr_efi_err("Failed to parse options\n");
   214				goto fail_free_cmdline;
   215			}
   216		}
   217	
   218		if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0) {
   219			status = efi_parse_options(cmdline_ptr);
 > 220			if (status != EFI_SUCCESS) {
   221				pr_efi_err("Failed to parse options\n");
   222				goto fail_free_cmdline;
   223			}
   224		}
   225	
   226		pr_efi("Booting Linux Kernel...\n");
   227	
   228		si = setup_graphics();
   229	
   230		status = handle_kernel_image(&image_addr, &image_size,
   231					     &reserve_addr,
   232					     &reserve_size,
   233					     dram_base, image);
   234		if (status != EFI_SUCCESS) {
   235			pr_efi_err("Failed to relocate kernel\n");
   236			goto fail_free_cmdline;
   237		}
   238	
   239		efi_retrieve_tpm2_eventlog();
   240	
   241		/* Ask the firmware to clear memory on unclean shutdown */
   242		efi_enable_reset_attack_mitigation();
   243	
   244		secure_boot = efi_get_secureboot();
   245	
   246		/*
   247		 * Unauthenticated device tree data is a security hazard, so ignore
   248		 * 'dtb=' unless UEFI Secure Boot is disabled.  We assume that secure
   249		 * boot is enabled if we can't determine its state.
   250		 */
   251		if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
   252		     secure_boot != efi_secureboot_mode_disabled) {
   253			if (strstr(cmdline_ptr, "dtb="))
   254				pr_efi("Ignoring DTB from command line.\n");
   255		} else {
   256			status = efi_load_dtb(image, &fdt_addr, &fdt_size);
   257	
   258			if (status != EFI_SUCCESS) {
   259				pr_efi_err("Failed to load device tree!\n");
   260				goto fail_free_image;
   261			}
   262		}
   263	
   264		if (fdt_addr) {
   265			pr_efi("Using DTB from command line\n");
   266		} else {
   267			/* Look for a device tree configuration table entry. */
   268			fdt_addr = (uintptr_t)get_fdt(&fdt_size);
   269			if (fdt_addr)
   270				pr_efi("Using DTB from configuration table\n");
   271		}
   272	
   273		if (!fdt_addr)
   274			pr_efi("Generating empty DTB\n");
   275	
   276		if (!efi_noinitrd) {
   277			max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
   278			status = efi_load_initrd(image, &initrd_addr, &initrd_size, max_addr);
   279			if (status != EFI_SUCCESS)
   280				pr_efi_err("Failed to load initrd!\n");
   281		}
   282	
   283		efi_random_get_seed();
   284	
   285		/*
   286		 * If the NX PE data feature is enabled in the properties table, we
   287		 * should take care not to create a virtual mapping that changes the
   288		 * relative placement of runtime services code and data regions, as
   289		 * they may belong to the same PE/COFF executable image in memory.
   290		 * The easiest way to achieve that is to simply use a 1:1 mapping.
   291		 */
   292		prop_tbl = get_efi_config_table(EFI_PROPERTIES_TABLE_GUID);
   293		flat_va_mapping = prop_tbl &&
   294				  (prop_tbl->memory_protection_attribute &
   295				   EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA);
   296	
   297		/* hibernation expects the runtime regions to stay in the same place */
   298		if (!IS_ENABLED(CONFIG_HIBERNATION) && !efi_nokaslr && !flat_va_mapping) {
   299			/*
   300			 * Randomize the base of the UEFI runtime services region.
   301			 * Preserve the 2 MB alignment of the region by taking a
   302			 * shift of 21 bit positions into account when scaling
   303			 * the headroom value using a 32-bit random value.
   304			 */
   305			static const u64 headroom = EFI_RT_VIRTUAL_LIMIT -
   306						    EFI_RT_VIRTUAL_BASE -
   307						    EFI_RT_VIRTUAL_SIZE;
   308			u32 rnd;
   309	
   310			status = efi_get_random_bytes(sizeof(rnd), (u8 *)&rnd);
   311			if (status == EFI_SUCCESS) {
   312				virtmap_base = EFI_RT_VIRTUAL_BASE +
   313					       (((headroom >> 21) * rnd) >> (32 - 21));
   314			}
   315		}
   316	
   317		install_memreserve_table();
   318	
   319		status = allocate_new_fdt_and_exit_boot(handle, &fdt_addr,
   320							efi_get_max_fdt_addr(dram_base),
   321							initrd_addr, initrd_size,
   322							cmdline_ptr, fdt_addr, fdt_size);
   323		if (status != EFI_SUCCESS)
   324			goto fail_free_initrd;
   325	
   326		efi_enter_kernel(image_addr, fdt_addr, fdt_totalsize((void *)fdt_addr));
   327		/* not reached */
   328	
   329	fail_free_initrd:
   330		pr_efi_err("Failed to update FDT and exit boot services\n");
   331	
   332		efi_free(initrd_size, initrd_addr);
   333		efi_free(fdt_size, fdt_addr);
   334	
   335	fail_free_image:
   336		efi_free(image_size, image_addr);
   337		efi_free(reserve_size, reserve_addr);
   338	fail_free_cmdline:
   339		free_screen_info(si);
   340		efi_free(cmdline_size, (unsigned long)cmdline_ptr);
   341	fail:
   342		return status;
   343	}
   344	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51805 bytes --]