efi_parse_options can fail if it is unable to allocate space for a copy
of the command line. Check the return value to make sure it succeeded.
Signed-off-by: Arvind Sankar <[email protected]>
---
drivers/firmware/efi/libstub/efi-stub.c | 18 ++++++++++++++----
drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++++--
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 930302d9415a..a4399537b4e6 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -207,11 +207,21 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
- cmdline_size == 0)
- efi_parse_options(CONFIG_CMDLINE);
+ cmdline_size == 0) {
+ status = efi_parse_options(CONFIG_CMDLINE);
+ if (status != EFI_SUCCESS) {
+ pr_efi_err("Failed to parse options\n");
+ goto fail_free_cmdline;
+ }
+ }
- if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0)
- efi_parse_options(cmdline_ptr);
+ if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0) {
+ status = efi_parse_options(cmdline_ptr);
+ if (status != EFI_SUCCESS) {
+ pr_efi_err("Failed to parse options\n");
+ goto fail_free_cmdline;
+ }
+ }
pr_efi("Booting Linux Kernel...\n");
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 0faba30d6406..ca549f26f45d 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -739,12 +739,20 @@ unsigned long efi_main(efi_handle_t handle,
}
#ifdef CONFIG_CMDLINE_BOOL
- efi_parse_options(CONFIG_CMDLINE);
+ status = efi_parse_options(CONFIG_CMDLINE);
+ if (status != EFI_SUCCESS) {
+ pr_efi_err("Failed to parse options\n");
+ goto fail;
+ }
#endif
if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
((u64)boot_params->ext_cmd_line_ptr << 32));
- efi_parse_options((char *)cmdline_paddr);
+ status = efi_parse_options((char *)cmdline_paddr);
+ if (status != EFI_SUCCESS) {
+ pr_efi_err("Failed to parse options\n");
+ goto fail;
+ }
}
/*
--
2.26.2
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 <[email protected]>
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/[email protected]