RE: [edk2] Should Path Name in File Path Media Device Path node be NULL terminated?
UEFI spec clearly say: "Path Name" is A NULL-terminated Path string including directory and file names. The length of this string n can be determined by subtracting 4 from the Length entry. A device path may contain one or more of these nodes. Each node can optionally add a "\" separator to the beginning and/or the end of the Path Name string. The complete path to a file can be found by logically concatenating all the Path Name strings in the File Path Media Device Path nodes. This is typically used to describe the directory path in one node, and the filename in another node. Thanks Feng -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Andrei Borzenkov Sent: Friday, February 24, 2017 3:06 PM To: edk2-devel-01 ; The development of GNU GRUB Subject: [edk2] Should Path Name in File Path Media Device Path node be NULL terminated? Historically grub2 built image paths using two File Path nodes - one for directory and one for file name relative to directory. These nodes had path names that were not NULL terminated. Recently we had bug report that secure boot using grub2 failed. It was tracked down to exactly the fact that paths were not NULL terminated. See http://git.savannah.gnu.org/cgit/grub.git/commit/?id=ce95549cc54b5d6f494608a7c390dba3aab4fba7 Unfortunately this caused another regression which looks like firmware truncating passed image path on first NULL https://bugzilla.opensuse.org/show_bug.cgi?id=1026344 Could someone clarify what is expected by EFI spec? Should each Path Name (even intermediate) be NULL terminated, or spec intends to say that only full path must be NULL terminated? ___ edk2-devel mailing list edk2-de...@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Should Path Name in File Path Media Device Path node be NULL terminated?
Historically grub2 built image paths using two File Path nodes - one for directory and one for file name relative to directory. These nodes had path names that were not NULL terminated. Recently we had bug report that secure boot using grub2 failed. It was tracked down to exactly the fact that paths were not NULL terminated. See http://git.savannah.gnu.org/cgit/grub.git/commit/?id=ce95549cc54b5d6f494608a7c390dba3aab4fba7 Unfortunately this caused another regression which looks like firmware truncating passed image path on first NULL https://bugzilla.opensuse.org/show_bug.cgi?id=1026344 Could someone clarify what is expected by EFI spec? Should each Path Name (even intermediate) be NULL terminated, or spec intends to say that only full path must be NULL terminated? ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH RFC] efi/chainloader: build image path as single File Path
According to EFI 9.3.6.4 File Path Media Device Path, Path Name is A NULL-terminated Path string including directory and file names. EFI chainloader is using two File Path nodes for image name - directory and file. It appears that at least some firmware implementations interpret NULL terminated directory path as end of full pathname, thus truncating it. OTOH leaving path names non-NULL terminated caused other errors (see commit ce95549cc54b5d6f494608a7c390dba3aab4fba7) Use single File Path node containing both directory and file name to avoid it. See also https://bugzilla.opensuse.org/show_bug.cgi?id=1026344 --- Anyone remembers why we build complete path using two nodes? As far as I can tell no other bootloader does it. I am really uneasy to do it now, but we have known bugs both with and without ce95549cc54b5d6f494608a7c390dba3aab4fba7 and this looks like the most straightforward fix. grub-core/loader/efi/chainloader.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c index adc8563..329dd57 100644 --- a/grub-core/loader/efi/chainloader.c +++ b/grub-core/loader/efi/chainloader.c @@ -158,29 +158,23 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename) d = GRUB_EFI_NEXT_DEVICE_PATH (d); } - /* File Path is NULL terminated. Allocate space for 2 extra characters */ - /* FIXME why we split path in two components? */ + /* File Path is NULL terminated. Allocate space for extra character */ file_path = grub_malloc (size - + ((grub_strlen (dir_start) + 2) + + ((grub_strlen (dir_start) + 1) * GRUB_MAX_UTF16_PER_UTF8 * sizeof (grub_efi_char16_t)) - + sizeof (grub_efi_file_path_device_path_t) * 2); + + sizeof (grub_efi_file_path_device_path_t)); if (! file_path) return 0; grub_memcpy (file_path, dp, size); - /* Fill the file path for the directory. */ + /* Fill the complete file path. */ d = (grub_efi_device_path_t *) ((char *) file_path + ((char *) d - (char *) dp)); grub_efi_print_device_path (d); copy_file_path ((grub_efi_file_path_device_path_t *) d, - dir_start, dir_end - dir_start); - - /* Fill the file path for the file. */ - d = GRUB_EFI_NEXT_DEVICE_PATH (d); - copy_file_path ((grub_efi_file_path_device_path_t *) d, - dir_end + 1, grub_strlen (dir_end + 1)); + dir_start, grub_strlen (dir_start)); /* Fill the end of device path nodes. */ d = GRUB_EFI_NEXT_DEVICE_PATH (d); -- tg: (2fb8cd2..) u/use-single-file-path-in-efi-chainloader (depends on: master) ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Loading DSDT table using 'acpi' or some memory write command?
Hi grub-devel, I'm endeavouring to pre-load a modified DSDT table using grub2. The syntax I've tried being as shown below, after which I chainload to Win10. This being done on a Dell E6540 with Win10 and grub 2.02 (or older.. tried many versions, same result). acpi /efi/dsdt.amlacpi --load-table dsdt /efi/dsdt.amlchainloader /efi/Boot/bootx64.efi Then I do 'acpidump -b' to peruse the DSDT table. In all cases, it remains the same system BIOS one, not my modified /efi/dsdt.aml one. I've also set 'set debug=all' and can see grub2 is reading my modified dsdt file and doing memory writes. So two questions: 1. Is 'acpi' not capable of loading a DSDT table for use with Win10? 2. if that is the case, is there some 'memload' or 'writefromfile' type command where I give a memory address and a file which grub2 can then write? The idea is simply to replace the existing DSDT at it's address with one given by the file of same size or smaller. Thank you,Nando ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] squash4: fix handling of fragments and sparse files
On Sat, Feb 18, 2017, 10:17 Andrei Borzenkov wrote:
> 1. Do not assume block list and fragment are mutually exclusive. Squash
> can pack file tail as fragment (unless -no-fragments is specified); so
> check read offset and read either from block list or from fragments as
> appropriate.
>
> 2. Support sparse files with zero blocks.
>
> 3. Fix fragment read - frag.offset is absolute fragment position,
> not offset relative to ino.chunk.
>
Go ahead.
>
> Reported and tested by Carlo Caione
>
> ---
>
> @Vladimir: we need regression tests for both of these cases. We could real
> small
> files only by accident - block list was zero, so it appeared to work.
>
How do you create those files reliably? Feel free to add any files to
grub-fs-tester. Feel free to commit without tests, we can add tests later.
>
> @Carlo, please test. I'm surprised it worked for you even without
> fragments as
> your file is sparse and grub immediately choked on the first zero block.
>
> grub-core/fs/squash4.c | 55
> +-
> 1 file changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/grub-core/fs/squash4.c b/grub-core/fs/squash4.c
> index b97b344..deee71a 100644
> --- a/grub-core/fs/squash4.c
> +++ b/grub-core/fs/squash4.c
> @@ -823,7 +823,12 @@ direct_read (struct grub_squash_data *data,
>curread = data->blksz - boff;
>if (curread > len)
> curread = len;
> - if (!(ino->block_sizes[i]
> + if (!ino->block_sizes[i])
> + {
> + /* Sparse block */
> + grub_memset (buf, '\0', curread);
> + }
> + else if (!(ino->block_sizes[i]
> & grub_cpu_to_le32_compile_time (SQUASH_BLOCK_UNCOMPRESSED)))
> {
> char *block;
> @@ -873,36 +878,57 @@ direct_read (struct grub_squash_data *data,
>
>
> static grub_ssize_t
> -grub_squash_read_data (struct grub_squash_data *data,
> - struct grub_squash_cache_inode *ino,
> - grub_off_t off, char *buf, grub_size_t len)
> +grub_squash_read (grub_file_t file, char *buf, grub_size_t len)
> {
> + struct grub_squash_data *data = file->data;
> + struct grub_squash_cache_inode *ino = &data->ino;
> + grub_off_t off = file->offset;
>grub_err_t err;
>grub_uint64_t a = 0, b;
>grub_uint32_t fragment = 0;
>int compressed = 0;
>struct grub_squash_frag_desc frag;
> + grub_off_t blk_len;
> + grub_uint64_t mask = grub_le_to_cpu32 (data->sb.block_size) - 1;
> + grub_size_t orig_len = len;
>
>switch (ino->ino.type)
> {
> case grub_cpu_to_le16_compile_time (SQUASH_TYPE_LONG_REGULAR):
> - a = grub_le_to_cpu64 (ino->ino.long_file.chunk);
>fragment = grub_le_to_cpu32 (ino->ino.long_file.fragment);
>break;
> case grub_cpu_to_le16_compile_time (SQUASH_TYPE_REGULAR):
> - a = grub_le_to_cpu32 (ino->ino.file.chunk);
>fragment = grub_le_to_cpu32 (ino->ino.file.fragment);
>break;
> }
>
> - if (fragment == 0x)
> -return direct_read (data, ino, off, buf, len);
> + /* Squash may pack file tail as fragment. So read initial part directly
> and
> + get tail from fragments */
> + blk_len = fragment == 0x ? file->size : file->size & ~mask;
> + if (off < blk_len)
> +{
> + grub_size_t read_len = blk_len - off;
> + grub_ssize_t res;
> +
> + if (read_len > len)
> + read_len = len;
> + res = direct_read (data, ino, off, buf, read_len);
> + if ((grub_size_t) res != read_len)
> + return -1; /* FIXME: is short read possible here? */
> + len -= read_len;
> + if (!len)
> + return read_len;
> + buf += read_len;
> + off = 0;
> +}
> + else
> +off -= blk_len;
>
>err = read_chunk (data, &frag, sizeof (frag),
> data->fragments, sizeof (frag) * fragment);
>if (err)
> return -1;
> - a += grub_le_to_cpu64 (frag.offset);
> + a = grub_le_to_cpu64 (frag.offset);
>compressed = !(frag.size & grub_cpu_to_le32_compile_time
> (SQUASH_BLOCK_UNCOMPRESSED));
>if (ino->ino.type == grub_cpu_to_le16_compile_time
> (SQUASH_TYPE_LONG_REGULAR))
> b = grub_le_to_cpu32 (ino->ino.long_file.offset) + off;
> @@ -943,16 +969,7 @@ grub_squash_read_data (struct grub_squash_data *data,
>if (err)
> return -1;
> }
> - return len;
> -}
> -
> -static grub_ssize_t
> -grub_squash_read (grub_file_t file, char *buf, grub_size_t len)
> -{
> - struct grub_squash_data *data = file->data;
> -
> - return grub_squash_read_data (data, &data->ino,
> - file->offset, buf, len);
> + return orig_len;
> }
>
> static grub_err_t
> --
> tg: (2fb8cd2..) u/squash-tail-fragment (depends on: master)
>
> ___
> Bug-grub mailing list
> bug-g...@gnu.org
> https://lists.gnu.org/mailman/listinfo/bug-grub
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https
