Linux-EFI Archive on lore.kernel.org
 help / color / Atom feed
From: kbuild test robot <lkp@intel.com>
To: Arvind Sankar <nivedita@alum.mit.edu>, Ard Biesheuvel <ardb@kernel.org>
Cc: kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
	linux-efi@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 10/10] efi/libstub: Check return value of efi_parse_options
Date: Thu, 30 Apr 2020 12:31:23 +0800
Message-ID: <202004301251.sicZQ3Nl%lkp@intel.com> (raw)
In-Reply-To: <20200429174120.1497212-12-nivedita@alum.mit.edu>


[-- 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 --]

  reply index

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar
2020-04-29 17:41 ` [PATCH 01/10] efi/x86: Use correct size for boot_params Arvind Sankar
2020-04-29 17:41 ` [PATCH 02/10] efi: Add a helper function to split 64-bit values Arvind Sankar
2020-04-29 23:51   ` kbuild test robot
2020-04-29 17:41 ` [PATCH 02/10] efi/libstub: " Arvind Sankar
2020-04-29 17:41 ` [PATCH 03/10] efi/x86: Use pr_efi_err for error messages Arvind Sankar
2020-04-29 18:47   ` Joe Perches
2020-04-29 18:49     ` Ard Biesheuvel
2020-04-29 18:57       ` Joe Perches
2020-04-29 18:59         ` Ard Biesheuvel
2020-04-29 19:47           ` Joe Perches
2020-04-29 19:48             ` Ard Biesheuvel
2020-04-29 21:43       ` Arvind Sankar
2020-04-29 21:45         ` Ard Biesheuvel
2020-04-29 21:51           ` Arvind Sankar
2020-04-29 21:53         ` Joe Perches
2020-04-29 21:55           ` Ard Biesheuvel
2020-04-29 22:20             ` Arvind Sankar
2020-04-30 15:14               ` Ard Biesheuvel
2020-04-29 17:41 ` [PATCH 04/10] efi/gop: " Arvind Sankar
2020-04-29 17:41 ` [PATCH 05/10] efi/tpm: " Arvind Sankar
2020-04-29 17:41 ` [PATCH 06/10] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar
2020-04-29 17:41 ` [PATCH 07/10] efi/libstub: Unify initrd loading across architectures Arvind Sankar
2020-04-29 17:41 ` [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading Arvind Sankar
2020-04-29 19:05   ` Ard Biesheuvel
2020-04-29 21:33     ` Arvind Sankar
2020-04-29 17:41 ` [PATCH 09/10] efi/x86: Support builtin command line Arvind Sankar
2020-04-29 19:07   ` Ard Biesheuvel
2020-04-29 21:39     ` Arvind Sankar
2020-04-29 21:40       ` Ard Biesheuvel
2020-04-29 21:48         ` Arvind Sankar
2020-04-29 21:51           ` Ard Biesheuvel
2020-04-29 17:41 ` [PATCH 10/10] efi/libstub: Check return value of efi_parse_options Arvind Sankar
2020-04-30  4:31   ` kbuild test robot [this message]
2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 01/11] efi/x86: Use correct size for boot_params Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 02/11] efi/libstub: Add a helper function to split 64-bit values Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 03/11] efi/libstub: Move pr_efi/pr_efi_err into efi namespace Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 04/11] efi/x86: Use efi_err for error messages Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 05/11] efi/gop: " Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 06/11] efi/tpm: " Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 07/11] efi/libstub: Upgrade ignored dtb= argument message to error Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 08/11] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 09/11] efi/libstub: Unify initrd loading across architectures Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 10/11] efi/x86: Support builtin command line Arvind Sankar
2020-04-30 18:28   ` [PATCH v2 11/11] efi/libstub: Check return value of efi_parse_options Arvind Sankar
2020-04-30 19:12   ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Joe Perches
2020-04-30 19:12     ` [PATCH 2/2] efi/libstub: Correct comment typos Joe Perches
2020-04-30 19:30       ` Ard Biesheuvel
2020-05-04 18:29         ` [trivial PATCH] efi/libstub: Reduce efi_printk object size Joe Perches
2020-05-05  7:50           ` Ard Biesheuvel
2020-05-05  8:01             ` Joe Perches
2020-04-30 19:29     ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Ard Biesheuvel
2020-04-30 19:38       ` Joe Perches
2020-04-30 20:40       ` Arvind Sankar
2020-04-30 20:42         ` Ard Biesheuvel
2020-05-04  8:17   ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202004301251.sicZQ3Nl%lkp@intel.com \
    --to=lkp@intel.com \
    --cc=ardb@kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nivedita@alum.mit.edu \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-EFI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-efi/0 linux-efi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-efi linux-efi/ https://lore.kernel.org/linux-efi \
		linux-efi@vger.kernel.org
	public-inbox-index linux-efi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-efi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git