[PATCH 4/8 v2] efi_loader: Introduce helper functions for EFI

[Ilias Apalodimas]

On Wed, Dec 30, 2020 at 08:29:54PM +0100, Heinrich Schuchardt wrote:
> On 12/30/20 4:07 PM, Ilias Apalodimas wrote:
> > A following patch introduces a different logic for loading initrd's

[...]

> > +struct load_file_info {
> > +	char dev[32];
> 
> if_typename_str[] contains all available strings. 8 bytes are long
> enough. The value can be validated with blk_get_device_by_str().
> 
> > +	char part[16];
> 
> Windows allows 128 partitions per device. I would not expect millions of
> devices. 8 bytes should be long enough.
> 
> > +	char filename[256];
> 
> This is a path not a file name. Please, adjust the parameter name.
> 
> In Windows the maximum length of a path is 260 characters. But does such
> a limit exist in UEFI?
> 
> In U-Boot we have:
> include/ext_common.h:33:#define EXT2_PATH_MAX 4096
> 
> So you cannot assume that the path is shorter then 4096 bytes.

This is defining something entirely differnt though. So I dont think we need
to adhere to any of these. 256 is perfectly sane for an initrd path.

> 
> I think the best thing to do is to use strdup() and then put 0x00 at the
> end of each string part. How about:
> 
> struct load_file_info {
> 	/*
> 	 * duplicated filespec, to be freed after usage
> 	 * 0x00 separated device, part, path
> 	 */
> 	char *filespec;
> 	char *part;
> 	char *filepath;
> }
> 
> So filespec would point to a buffer containing:
> 
> device\0   part\0   \path\file\0    \0
> |          |        |- filepath points here
> |          |- part points here
> |- filespec points here
> 

I generally try to avoid functions that need free() after the usagem since
they tend to be error prone. 
In any case this whole thing we probably go away if we just store a device
path in initrd. So let me have a look and see were we'll end up.

[...]

Cheers
/Ilias