[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 04/19] mm: assert that we preserve header vs region alignment
From: |
Daniel Kiper |
Subject: |
Re: [PATCH 04/19] mm: assert that we preserve header vs region alignment |
Date: |
Wed, 20 Oct 2021 19:43:13 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Tue, Oct 12, 2021 at 06:29:53PM +1100, Daniel Axtens wrote:
> grub_mm_region_init() does:
>
> h = (grub_mm_header_t) (r + 1);
>
> where h is a grub_mm_header_t and r is a grub_mm_region_t.
>
> Cells are supposed to be GRUB_MM_ALIGN aligned, but while grub_mm_dump
> ensures this vs the region header, grub_mm_region_init() does not.
>
> It's better to be explicit than implicit here: rather than changing
> grub_mm_region_init() to ALIGN_UP(), require that the struct is
> explictly a multiple of the header size.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
> include/grub/mm_private.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/grub/mm_private.h b/include/grub/mm_private.h
> index e80a059dd4e4..533b47173e18 100644
> --- a/include/grub/mm_private.h
> +++ b/include/grub/mm_private.h
> @@ -20,6 +20,7 @@
> #define GRUB_MM_PRIVATE_H 1
>
> #include <grub/mm.h>
> +#include <grub/misc.h>
>
> /* Magic words. */
> #define GRUB_MM_FREE_MAGIC 0x2d3c2808
> @@ -82,4 +83,11 @@ typedef struct grub_mm_region
> extern grub_mm_region_t EXPORT_VAR (grub_mm_base);
> #endif
>
> +static inline void grub_mm_size_sanity_check(void) {
static inline void
grub_mm_size_sanity_check (void)
{
> + /* Ensure we preserve alignment when doing h = (grub_mm_header_t) (r + 1)
> */
> + COMPILE_TIME_ASSERT((sizeof(struct grub_mm_region) %
Missing spaces before "(" above and below...
s/struct grub_mm_region/grub_mm_region_t/
s/struct grub_mm_header/grub_mm_header_t/
> + sizeof(struct grub_mm_header)) == 0);
I think this check is insufficient. The GRUB_MM_ALIGN should be checked
somehow too. E.g. if sizeof(struct grub_mm_region) == 16,
sizeof(struct grub_mm_header) == 8 and GRUB_MM_ALIGN == 32 then assert
above will pass but the GRUB will blow up later. Of course numbers are not
real but I chose them to show the problem. Hmmm... If we could choose
grub_mm_region_t and grub_mm_header_t sizes properly then probably we
could drop GRUB_MM_ALIGN...
Daniel
- [PATCH 00/19] Requesting more memory from firmware, Daniel Axtens, 2021/10/12
- [PATCH 03/19] mm: document grub internal memory management structures, Daniel Axtens, 2021/10/12
- [PATCH 01/19] grub-shell: Boot PowerPC using PMU instead of CUDA for power management, Daniel Axtens, 2021/10/12
- [PATCH 04/19] mm: assert that we preserve header vs region alignment, Daniel Axtens, 2021/10/12
- Re: [PATCH 04/19] mm: assert that we preserve header vs region alignment,
Daniel Kiper <=
- [PATCH 02/19] grub-shell: pseries: don't pass fw_opt to qemu, Daniel Axtens, 2021/10/12
- [PATCH 05/19] mm: when adding a region, merge with region after as well as before, Daniel Axtens, 2021/10/12
- [PATCH 06/19] configure: properly pass through MM_DEBUG, Daniel Axtens, 2021/10/12
- [PATCH 07/19] Add memtool module with memory allocation stress-test, Daniel Axtens, 2021/10/12
- [PATCH 08/19] mm: Drop unused unloading of modules on OOM, Daniel Axtens, 2021/10/12
- [PATCH 10/19] efi: mm: Always request a fixed number of pages on init, Daniel Axtens, 2021/10/12
- [PATCH 12/19] efi: mm: Pass up errors from `add_memory_regions ()`, Daniel Axtens, 2021/10/12