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

[Ilias Apalodimas]

On Fri, 21 Feb 2025 at 21:22, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> 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.

Fair enough. I am not against this patch, I am just to figure out if
we can restrict what we preserve a bit more

Thanks
/Ilias
>
> 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
>