[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V3 2/3] Add support for avoiding firmware in relocations

From: Vladimir 'φ-coder/phcoder' Serbinenko
Subject: Re: [PATCH V3 2/3] Add support for avoiding firmware in relocations
Date: Wed, 08 Feb 2012 20:41:11 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20120104 Icedove/8.0

On 08.02.2012 17:55, Matthew Garrett wrote:
EFI and OF both support firmware regions which may be in use during loading.

+grub_relocator_firmware_overlaps (grub_phys_addr_t target, grub_size_t size)
+  grub_efi_uintn_t mmapsize = 0, desc_size = 0;
+  grub_efi_uint32_t descriptor_version = 0;
+  grub_efi_memory_descriptor_t *descs = NULL;
+  grub_efi_uintn_t key;
+  int overlap = 0;
+  grub_efi_memory_descriptor_t *desc;
+  grub_efi_get_memory_map (&mmapsize, NULL,&key,&desc_size,
+                       &descriptor_version);
You need to check return value.
You don't use key. You can just make it NULL (grub_efi_get_memory_map handles it).
+  descs = grub_malloc (mmapsize);
+  if (!descs)
+    return 0;
You seem to lack error handling as whole. I'd suggest make this function return grub_err_t and generate a grub_error itself if overlap is detected.
You current code would happily allocate anywhere if grub_malloc fails.
+  grub_efi_get_memory_map (&mmapsize, descs,&key,&desc_size,
+                       &descriptor_version);
    adjust_limits (rel,&min_addr,&max_addr, target, target);

+  if (avoid_firmware)
+    {
+      if (grub_relocator_firmware_overlaps(target, size))
+      return grub_error(GRUB_ERR_BAD_ARGUMENT, "target overlaps with 
+    }
Ditto. Also here you have indentation problem

Vladimir 'φ-coder/phcoder' Serbinenko

reply via email to

[Prev in Thread] Current Thread [Next in Thread]