Re: [PATCH] misra: avoid unsafe cast of __init_begin

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/10/2025 5:45 pm, Dmytro Prokopchuk1 wrote:
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index eb8ed19ca1..00c4c8832d 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -481,7 +481,7 @@ void free_init_memory(void)
>      unsigned long len = __init_end - __init_begin;
>      uint32_t insn;
>      unsigned int i, nr = len / sizeof(insn);
> -    uint32_t *p;
> +    uint8_t *p;
>      int rc;
>  
>      rc = modify_xen_mappings((unsigned long)__init_begin,
> @@ -501,9 +501,11 @@ void free_init_memory(void)
>  #else
>      insn = AARCH64_BREAK_FAULT;
>  #endif
> -    p = (uint32_t *)__init_begin;
>      for ( i = 0; i < nr; i++ )
> -        *(p + i) = insn;
> +    {
> +        p = (uint8_t *)__init_begin + i * sizeof(insn);
> +        memcpy(p, &insn, sizeof(insn));
> +    }
>  
>      rc = destroy_xen_mappings((unsigned long)__init_begin,
>                                (unsigned long)__init_end);

I'm in agreement with Eclair, this is horrible code.

Putting an undefined instruction here is pretty useless.  By the time
destroy_xen_mappings() completes, you'll suffer a pagefault from trying
to execute there, rather than actually getting to execute code.

On x86 we simply zero the memory and then hand it back to the heap
allocator.  ARM doesn't seem to do this yet.

Irrespective, it either wants zeroing, or maybe SCRUB_PATTERN as we use
for other invalid memory.

Both of these can be done with a simple memset(), which simplifies
everything.

~Andrew