Re: [PATCH v4 5/5] blkmap: add pmem nodes for blkmap memory mapped slices

[Heinrich Schuchardt]

Am 21. Februar 2025 19:55:03 MEZ schrieb Ilias Apalodimas <ilias.apalodimas at linaro.org>:
>Hi Sughosh
>
>This generally looks ok, but I don't love the idea of unconditionally
>preserving all slices regardless of their usage.
>Basically, if a user doesn't unmap that slice it will end in kernel
>memory. My fear is that someone will forget device sensitive data in a
>blkmap....

I don't that you can easily determine which blkmap and which blkmap slice was created why.

But can we realisticly assume that a device that U-Boot reads from is secure against reading at kernel runtime. I don't believe so as U-Boot runs at the same exception level as the kernel. U-Boot and the kernel do not empty RAM. You always have to assume that whatever is in U-Boot memory is dicoverable at kernel runtime.

If you load sensitive data, you must overwrite it before booting.

Best regards

Heinrich



>
>On Mon, 3 Feb 2025 at 12:59, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>>
>> The EFI HTTP boot puts the ISO installer image at some location in
>> memory which needs to be added to the devicetree as persistent
>> memory (pmem) node. The OS installer then gets information about the
>> presence of this ISO image through the pmem node and proceeds with the
>> installation.
>>
>> In U-Boot, this ISO image gets mounted as a blkmap device, with a
>> memory mapped slice. Add a helper function which iterates through all
>> such memory mapped blkmap slices, and calls the FDT fixup function to
>> add the pmem node. Invoke this helper function as part of the DT fixup
>> which happens before booting the OS.
>>
>> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
>> ---
>> Changes since V3:
>> * Move the definition of the helper function to the efi_helper.c
>> * Remove the region of the blkmap mem map device from the EFI memory
>>   map along with adding the pmem node
>>
>
>[...]
>
>> @@ -680,3 +683,52 @@ out:
>>
>>         return ret;
>>  }
>> +
>> +static int add_blkmap_pmem_nodes(void *fdt, struct blkmap *bm)
>> +{
>> +       int ret;
>> +       u32 size;
>> +       ulong addr;
>> +       efi_status_t status;
>> +       struct blkmap_mem *bmm;
>> +       struct blkmap_slice *bms;
>> +       struct blk_desc *bd = dev_get_uclass_plat(bm->blk);
>> +
>> +       list_for_each_entry(bms, &bm->slices, node) {
>> +               if (bms->type != BLKMAP_SLICE_MEM)
>> +                       continue;
>
>Can we convert the 'type' to 'preserve' and teach
>blkmap_create_ramdisk() to pass that flag?
>This way we can unconditionally pass it from EFI HTTP installers, and
>let the command line users decide if they want to preserve it.
>
>
>> +
>> +               bmm = container_of(bms, struct blkmap_mem, slice);
>> +
>> +               addr = (ulong)(uintptr_t)bmm->addr;
>> +               size = (u32)bms->blkcnt << bd->log2blksz;
>> +
>> +               ret = fdt_fixup_pmem_region(fdt, addr, size);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               status = efi_remove_memory_map(addr, size,
>> +                                              EFI_CONVENTIONAL_MEMORY);
>> +               if (status != EFI_SUCCESS)
>> +                       return -1;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>
>
>Thanks
>/Ilias