Re: [PATCH] fs/iso9660.c: Correct error message for missing UUID

2024-07-01 Thread Thomas Schmitt via Grub-devel
Hi, sorry for goofing up "git send-email" again. This time not by omitting the empty line after the subject of a cover letter but probably by leaving the commit id of the previous "git format-patch" in the "git send-email" command line. git send-email --to=grub-devel@gnu.org $directory

[PATCH] fs/iso9660.c: Correct error message for missing UUID

2024-07-01 Thread Thomas Schmitt via Grub-devel
The UUID emitted by function grub_iso9660_uuid is derived from the ISO 9660 Primary Volume Descriptor field "Volume Modification Date and Time". But the error message about possible invalid content of this field talks of "creation date" rather than of "modification date

[PATCH] fs/iso9660.c: Correct error message for missing UUID

2024-07-01 Thread Thomas Schmitt via Grub-devel
The UUID emitted by function grub_iso9660_uuid is derived from the ISO 9660 Primary Volume Descriptor field "Volume Modification Date and Time". But the error message about possible invalid content of this field talks of "creation date" rather than of "modification date

Re: How to test grub_iso9660_uuid from userland ?

2024-07-01 Thread Thomas Schmitt via Grub-devel
Hi, the question is now: Is it worth to correct an error text which i cannot get to show up ? Vladimir 'phcoder' Serbinenko wrote: > grub-fstest IMAGE ls -- -l $ gunzip /tmp/iso9660_early_ce.iso $ ./grub-fstest /tmp/iso9660_early_ce.iso ls -- -l ... Device loop0: Filesystem type

How to test grub_iso9660_uuid from userland ?

2024-07-01 Thread Thomas Schmitt via Grub-devel
Hi, i found a wrong word in an error message of grub-core/fs/iso9660.c function grub_iso9660_uuid(): grub_error (GRUB_ERR_BAD_NUMBER, "no creation date in filesystem to generate UUID"); The missing entity would be the modification date data->voldesc.modified rather than the creation

Re: [PATCH 0/2] grub-fstest: Show error message if command causes grub_errno

2024-06-21 Thread Thomas Schmitt via Grub-devel
Hi, somehow i managed to omit the first paragraph of the cover letter: - grub-fstest describes itself as "debug tool for filesystem driver" but fails to forward to the user the grub_errno status and grub_errmsg of the drivers

[PATCH 0/2] grub-fstest: Show error message if command causes grub_errno

2024-06-20 Thread Thomas Schmitt via Grub-devel
40450278223161.luks2.NWP' not found. UUID FAIL 8a1f1e83695d4dd1b6b73fcb542b73da - grub_func_test reports checksum problems tests/video_checksum.c:checksum:615: assert failed: 0 Checksum cmdline_cat_2560x1440xrgba:44 failed: 0x62031fea vs 0x8071678a -

[PATCH 2/2] tests: Use new grub-fstest option -E with iso9660_ce_loop test

2024-06-20 Thread Thomas Schmitt via Grub-devel
-fstest on iso9660_ce_loop.iso and iso9660_ce_loop2.iso. This suppresses the error message and the non-zero exit value. Signed-off-by: Thomas Schmitt --- tests/iso9660_test.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/iso9660_test.in b/tests/iso9660_test.in index

[PATCH 1/2] grub-fstest: Show error message if command causes grub_errno

2024-06-20 Thread Thomas Schmitt via Grub-devel
re-enables the old behavior which did not show grub_errmsg and did not exit with non-zero value if grub_errno was found not being GRUB_ERR_NONE. Signed-off-by: Thomas Schmitt --- util/grub-fstest.c | 30 +- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git

[PATCH 1/1] util/grub-mkrescue: Check existence of option arguments

2024-06-17 Thread Thomas Schmitt via Grub-devel
older versions of grub-mkrescue did with a missing argument (e.g 2.02). Fixes: https://savannah.gnu.org/bugs/index.php?65880 Signed-off-by: Thomas Schmitt --- util/grub-mkrescue.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/util/grub-mkrescue.c b/util/grub-mkrescue.c index abcc1c2f5

Re: [PATCH v2] util/grub-mkrescue: use capitalised paths for removable EFI images

2024-06-16 Thread Thomas Schmitt via Grub-devel
Hi, now that i know how to test grub-mkrescue out of the git clone, i also gave your patch v2 a run. Its mail form seems to be problematic: - Alpine shows two leading blanks in the context lines instead of one. Empty context lines show no leading blank. Long lines show intermediate line

[Solved] Re: How to test the git clone without "make install" ?

2024-06-14 Thread Thomas Schmitt via Grub-devel
Hi, i wrote: > > So i will start a new thread with the question: > > How do i convince the git clone to produce programs and ISO for 64 bit > > EFI. Vladimir 'phcoder' Serbinenko wrote: > ./configure --with-platform=efi --target=x86_64 Fast and concise as ever. :)) Thanks, Vladimir. It works

[Solved] Re: How to test the git clone without "make install" ?

2024-06-14 Thread Thomas Schmitt via Grub-devel
Hi, Maximilian Stendler wrote: > to keep the host installation clean, I would probably use a container. Yes, a virtual machine came to my mind. Easy to clone and to dispose. But there must be some better way to test a utility built from git independenly of systemwide directories. Vladimir

How to test the git clone without "make install" ?

2024-06-14 Thread Thomas Schmitt via Grub-devel
Hi, on occasion of https://savannah.gnu.org/bugs/index.php?65880 "heap-buffer-overflow in grub-mkrescue.c" i try to get grub-mkrescue running from git. My problem is that grub_util_get_pkglibdir() returns /usr/local/lib/grub and grub_util_get_pkgdatadir() returns /usr/local/share/grub

Re: [PATCH] util/grub-mkrescue: use capitalised paths for removable EFI images

2024-06-13 Thread Thomas Schmitt via Grub-devel
Hi, Mingcong Bai wrote: > I was testing for loongarch64-efi. As noted in the commit message, I > found that Loongson's firmware incapable of handling non-upper-case EFI > boot paths If no test reports emerge about other platforms, then i would consider to reduce the patch to what was tested and

Re: [PATCH] util/grub-mkrescue: use capitalised paths for removable EFI images

2024-06-13 Thread Thomas Schmitt via Grub-devel
Hi, Mingcong Bai wrote: > Thanks for your review. I will now submit v2. Well, it's not actually a review but rather pointing out a problem which would probably cause a failure of the xorriso run, when the option -hfs-bless-by i /System/Library/CoreServices/boot.efi does not find "boot.efi"

Re: [PATCH] util/grub-mkrescue: use capitalised paths for removable EFI images

2024-06-11 Thread Thomas Schmitt via Grub-devel
Hi, Mingcong Bai wrote: > While FAT < 32 filesystems are not case sensitive (which grub-mkrescue > creates > as a FAT12 image via mformat with a size of 2.88MiB), it seems that > some of Loongson's LoongArch-based firmware (namely those found on their > latest XA61200 boards) seems to treat this

Re: [PATCH] util/grub-mkrescue: specify -iso-level 3 in xorriso options

2023-09-20 Thread Thomas Schmitt
Hi, Gerd Hoffmann wrote: > I doubt the vfat driver of the uefi firmware supports rock ride, > and that is the driver used to load efi/boot/bootloongarch64.efi > because grub is not yet loaded ... xorrisofs option -iso-level 3 has no effect on the FAT filesystem in the EFI boot image (/efi.img

Re: [PATCH] util/grub-mkrescue: specify -iso-level 3 in xorriso options

2023-09-20 Thread Thomas Schmitt
Hi, Mingcong Bai wrote: > This is needed for architectures whose EFI executables has file names that > exceeds the 8.3 (extension.suffix) convention, such as LoongArch64 > (bootloongarch64.efi). In general -iso-level 3 is a harmless setting if the reading operating system is Linux or

Re: [PATCH 0/2] fs/iso9660: Delay CE hop until end of current SUSP area

2023-03-30 Thread Thomas Schmitt
Hi, Daniel Kiper wrote: > AIUI this [1] email presented some concerns. > [1] https://lists.gnu.org/archive/html/grub-devel/2023-03/msg00022.html This was about the trailing blank in the output of grub-fstest. I decided to simply remove it because it is not expectable that any wrong handling of

[PATCH 2/2] tests: Add test for iso9660 delayed CE hop

2023-03-07 Thread Thomas Schmitt
-fstest. Signed-off-by: Thomas Schmitt --- tests/iso9660_early_ce.iso.gz | Bin 0 -> 709 bytes tests/iso9660_test.in | 24 2 files changed, 24 insertions(+) create mode 100644 tests/iso9660_early_ce.iso.gz diff --git a/tests/iso9660_early_ce.iso.gz b/tests/iso9660_e

[PATCH 1/2] fs/iso9660: Delay CE hop until end of current SUSP area

2023-03-07 Thread Thomas Schmitt
and perform checks and reading of new data only after the reader loop has ended. Signed-off-by: Thomas Schmitt --- grub-core/fs/iso9660.c | 84 ++ 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c

[PATCH 0/2] fs/iso9660: Delay CE hop until end of current SUSP area

2023-03-07 Thread Thomas Schmitt
sequence dd conv=notrunc if=ce_field bs=1 seek=37198 of="$iso" dd conv=notrunc if=nm_field bs=1 seek=37226 of="$iso" - Have a nice day :) Thomas Thomas Schmitt (2): fs/iso9660: Delay CE hop until end of current SUSP area tests: Add te

Is the last blank in the output of grub-fstester a reliable feature ?

2023-03-05 Thread Thomas Schmitt
Hi, i am working on a patch to let GRUB delay the execution of a ISO 9660 SUSP CE hop until the current SUSP area is consumed. Currently it hops immediately which is wrongly relying on implementation details of the ISO producer. SUSP 1.12 specs are clear. Linux obeys them. The code works but the

Re: [PATCH v3] tests: Add pathological iso9660 filesystem tests

2023-02-16 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > > > +echo "Testing for proper recognition of CE loops ... " > Ugh, this should be "echo -n". My make "check" wrote: > > Testing for proper recognition of CE loops ... > > FAIL (iso9660_ce_loop) > > FAIL iso9660_test (exit status: 1) Actually this looks quite ok

Re: [PATCH 2/2] docs: Document that the functional test requires the package xfonts-unifont

2023-02-16 Thread Thomas Schmitt
r the functional tests > > * If running a Linux kernel the following modules must be loaded: >- fuse, loop Reviewed-by: Thomas Schmitt Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/2] tests: Return hard error for functional test when unicode.pf2 does not exist

2023-02-16 Thread Thomas Schmitt
t; > +if [ ! -e "@builddir@/"unicode.pf2 ]; then > + echo "Functional test requires grub-mkfont support" > + exit 99 > +fi > + > case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in > # PLATFORM: Max RAM is 256M > mips

Re: [PATCH v3] tests: Add pathological iso9660 filesystem tests

2023-02-16 Thread Thomas Schmitt
e > +"@builddir@/grub-fs-tester" rockridge_joliet_1999 > + > +echo "Testing for proper recognition of CE loops ... " > +for fs in iso9660_ce_loop iso9660_ce_loop2; do > + tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date > '+%Y%m%d%H%M%S%N').${fs}

Re: make check: grub-mkrescue fails missing unicode.pf2. Leftovers in /tmp

2023-02-15 Thread Thomas Schmitt
Hi, another confused statement of mine: > Unicode is mentioned two times in INSTALL. I meant "Unifont" and "unifont". Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel

Re: make check: grub-mkrescue fails missing unicode.pf2. Leftovers in /tmp

2023-02-15 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > IIRC, xfonts-unifont is pulled in by the unifont package on Debian, but > it is the one with the files you need. No, its not documented, but then > not every single debian package name needed is documented because the > documentation is partially distribution agnostic.

Re: make check: grub-mkrescue fails missing unicode.pf2. Leftovers in /tmp

2023-02-14 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > You have the package xfonts-unifont installed correct? Urm, no. (I have no desktop running. Just fvwm.) Is the need for this package documented somewhere ? Well, now it is installed. I do $ ./configure ... GRUB2 will be compiled with following components:

Re: make check: grub-mkrescue fails missing unicode.pf2. Leftovers in /tmp

2023-02-13 Thread Thomas Schmitt
Hi, i have to correct a confused statement of mine: > I suspect that ${files} comes from the "--files=" argument that is given > to ./grub-shell by tests/util/grub-shell.in. The "--files=" argument is given by tests/grub_func_test.in. Meanwhile i verified that this argument is the origin of

Re: make check: grub-mkrescue fails missing unicode.pf2. Leftovers in /tmp

2023-02-13 Thread Thomas Schmitt
Hi, Vladimir Serbinenko wrote: > Look at the end of configure message. Is free type enabled? Was unifont > found? $ ./configure ... checking whether -Wtrampolines work... yes checking for freetype2... yes checking ft2build.h

make check: grub-mkrescue fails missing unicode.pf2. Leftovers in /tmp

2023-02-10 Thread Thomas Schmitt
Hi, i encountered some stumblestones after running "make check": - I see in file ./test-suite.log : FAIL: grub_func_test xorriso 1.5.2 : RockRidge filesystem manipulator, libburnia project. ...

Re: [PATCH v2] tests: Add pathological iso9660 filesystem tests

2023-02-10 Thread Thomas Schmitt
iso9660_ce_loop iso9660_ce_loop2; do > + tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date > '+%Y%m%d%H%M%S%N').${fs}.XXX"` || > +{ echo "Failed to make temporary directory"; exit 99; } > + gunzip <"$srcdir"/tests/${fs}.iso.gz >"${temp

Re: [PATCH] tests: Add pathological iso9660 filesystem tests

2023-02-05 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > --- a/tests/iso9660_test.in > +++ b/tests/iso9660_test.in > @@ -12,4 +12,13 @@ fi > "@builddir@/grub-fs-tester" rockridge_joliet > "@builddir@/grub-fs-tester" joliet_1999 > "@builddir@/grub-fs-tester" rockridge_1999 > -"@builddir@/grub-fs-tester"

Re: [PATCH v3 0/5] fs/iso9660: Fix out-of-bounds read

2023-01-28 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > Why does only one suffice? It > sounds like they test different code paths. Is it possible that there > is a future code regression such that one iso succeeds and the other > fails? They follow different code paths before hunk 4 of patch 5 fixes the bug that CE and ST

Re: [PATCH v3 0/5] fs/iso9660: Fix out-of-bounds read

2023-01-27 Thread Thomas Schmitt
Hi, i wrote: > > So it would be better to add one or two canned images: > > 897 bytes of http://scdbackup.webframe.org/ce_loop.iso.gz > > 904 bytes of http://scdbackup.webframe.org/ce_loop2.iso.gz Glenn Washburn wrote: > These are small, I'm good with adding these images to the repository.

Re: [PATCH v3 0/5] fs/iso9660: Fix out-of-bounds read

2023-01-25 Thread Thomas Schmitt
Hi, Daniel Kiper wrote: > Thomas, it would be nice if you could add the broken ISOs images which you > used for tests to the tests in the GRUB. If you do that please CC Glenn. Is it wise to have a test which will loop endlessly in case of failure ? Is there a way to let a test time out ?

Re: [PATCH v3 5/5] fs/iso9660: Prevent skipping CE or ST at start of continuation area

2023-01-21 Thread Thomas Schmitt
SUSP entries of the file. In case of ST this could > cause following non-SUSP bytes to be interpreted as SUSP entries. > > Signed-off-by: Thomas Schmitt > Tested-by: Lidong Chen > --- > grub-core/fs/iso9660.c | 16 > 1 file changed, 16 insertions(+) > >

Re: [PATCH v2 0/5] fs/iso9660: Fix out-of-bounds read

2023-01-20 Thread Thomas Schmitt
Hi, Lidong Chen wrote: > I ran grub-fastest with both ce_loop ISO files. The endless loops were > detected and Grub exited accordingly. Good. > I didn't know where the grub error message > were stored in case of grub-fastest. So you don't see an error message ? I had the same problem a while

Re: [PATCH v2 0/5] fs/iso9660: Fix out-of-bounds read

2023-01-19 Thread Thomas Schmitt
Hi, i wrote: > > libisofs and xorriso are in the process of getting an adjustable curb to > > prevent the production of ISO filesystems with files which would not show > > up in Linux. I decided for 100,000 hops as hard limit but set the default > > to 31. Lidong Chen wrote: > I am not sure I

Re: [PATCH v2 0/5] fs/iso9660: Fix out-of-bounds read

2023-01-18 Thread Thomas Schmitt
Hi, On Wed, 18 Jan 2023 08:23:53 + Lidong Chen wrote: > Thomas also pointed out the issue of the potential endless > loops by CE. Since the sugguested fix requires a bit more > investigation, and as Thomas pointed out that it should be > handled in a separate patch, the fix is not included

Re: [PATCH v2 5/5] fs/iso9660: Prevent skipping CE or ST at start of continuation area

2023-01-18 Thread Thomas Schmitt
SUSP entries of the file. In case of ST this could > cause following non-SUSP bytes to be interpreted as SUSP entries. > > Signed-off-by: Thomas Schmitt scdbac...@gmx.net > --- > grub-core/fs/iso9660.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/g

Re: [PATCH v2 4/5] fs/iso9660: Incorrect check for entry boundary

2023-01-18 Thread Thomas Schmitt
zeof (*entry) to check the entry boundary. > > Signed-off-by: Lidong Chen > Reviewed-by: Thomas Schmitt > --- > grub-core/fs/iso9660.c | 17 +++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c

Re: [PATCH v2 3/5] fs/iso9660: Avoid reading past the entry boundary

2023-01-18 Thread Thomas Schmitt
Hi, On Wed, 18 Jan 2023 08:23:56 + Lidong Chen wrote: > Added a check for the SP entry data boundary before reading it. > > Signed-off-by: Lidong Chen > Reviewed-by: Thomas Schmitt > --- > grub-core/fs/iso9660.c | 16 ++-- > 1 file changed, 14 inser

Re: [PATCH v2 2/5] fs/iso9660: Prevent read past the end of system use area

2023-01-18 Thread Thomas Schmitt
grub_free (sua); > sua = grub_malloc (sua_size); > if (!sua) > @@ -319,6 +338,11 @@ grub_iso9660_susp_iterate (grub_fshelp_node_t node, > grub_off_t off, > grub_free (sua); > return 0; > } > + > + entry = (struct grub_iso9660_susp_e

Re: [PATCH v2 1/5] fs/iso9660: Add check to prevent infinite loop

2023-01-18 Thread Thomas Schmitt
nd of the block and ensure the read is within directory > boundary. > > Signed-off-by: Lidong Chen > Reviewed-by: Thomas Schmitt > --- > grub-core/fs/iso9660.c | 21 + > 1 file changed, 21 insertions(+) > > diff --git a/grub-core/fs/iso9660.c b/grub-core

Re: Proposal v2: fs/iso9660: Prevent skipping CE or ST at start of continuation area

2023-01-12 Thread Thomas Schmitt
Hi, Lidong Chen wrote: > To test it, I am thinking to add the ISO entry in 40_custom script, then > select > the ISO from Grub menu. Is it the right way to test it? Or, is there a better > way > to it? I have to leave the answer to the experienced GRUB developers. Testing is my weak spot with

Re: Proposal v2: fs/iso9660: Prevent skipping CE or ST at start of continuation area

2023-01-11 Thread Thomas Schmitt
Hi, i created another bad ISO which i expect to lead to an endless loop in existing GRUB (i.e. before applying the proposed change). Both ISOs can be downloaded as gzip-compressed files now: http://scdbackup.webframe.org/ce_loop.iso.gz SHA256:

Re: Proposal v2: fs/iso9660: Prevent skipping CE or ST at start of continuation area

2023-01-09 Thread Thomas Schmitt
Hi, Lidong Chen wrote: > Thanks for the clarification. I created a new patch for the fix and added > you as the “Signed-off-by”. > My question is how to test it. I just sent libisofs into an endless recursion loop. Grrr. For this i created a small ISO with a file which surely needs a CE, changed

Re: Proposal v2: fs/iso9660: Prevent skipping CE or ST at start of continuation area

2023-01-06 Thread Thomas Schmitt
Hi, i wrote on Fri, 16 Dec 2022 10:42:13 +0100: > > If processing of a SUSP CE entry leads to a continuation area which begins > > by entry CE or ST, then these entries were skipped without interpretation. > > In case of CE this would lead to premature end of processing the SUSP > > entries of

Re: [RFC PATCH] kern/dl: Add module version check

2022-12-21 Thread Thomas Schmitt
Hi, Pete Batard wrote: > > unlike what is the case for UEFI, one can not expect > > to be able to pick all the GRUB core files needed to convert a GRUB > > based ISO bootable media to a GRUB based USB bootable media [...] > > Typically, one of the > > missing files will be a 'core.img' that can

Re: [PATCH 4/4] fs/iso9660: Incorrect check for entry boudary

2022-12-20 Thread Thomas Schmitt
ec 15, 2022, at 10:20 AM, Thomas Schmitt wrote: > > This loop is iterating over component records, not SUSP entries. > > Minimum size of a component record is 2. > > > > So i think the appropriate condition is: > > > > while (pos + 1 < entry->

Re: [PATCH 1/4] fs/iso9660: Add check to prevent infinite loop

2022-12-19 Thread Thomas Schmitt
Hi, i wrote: > > (Are we aware of the file size limit of 32 GiB - 14 KiB - 1 imposed by > > struct grub_fshelp_node { ... struct grub_iso9660_dir dirents[8]; ... } > > ? ) Lidong Chen wrote: > I am not familiar with this file size limit. Do we need to add a check > somewhere? Good question.

Proposal v2: fs/iso9660: Prevent skipping CE or ST at start of continuation area

2022-12-16 Thread Thomas Schmitt
of the file. In case of ST this could cause following non-SUSP bytes to be interpreted as SUSP entries. Signed-off-by: Thomas Schmitt --- grub-core/fs/iso9660.c.lidong_chen_patch_2 2022-12-15 11:46:50.654295924 +0100 +++ grub-core/fs/iso9660.c.lidong_chen_patch_2_ce_fix_v22022-12-16 13

Proposal: fs/iso9660: Prevent skipping CE or ST at start of continuation area

2022-12-16 Thread Thomas Schmitt
were skipped without interpretation. In case of CE this would lead to premature end of processing the SUSP entries of the file. In case of ST this could cause following non-SUSP bytes to be interpreted as SUSP entries. Signed-off-by: Thomas Schmitt --- grub-core/fs/iso9660.c.lidong_chen_patch

Re: [PATCH 2/4] fs/iso9660: Prevent read past the end of system use area

2022-12-16 Thread Thomas Schmitt
Hi, while preparing a proposal how to avoid skipping of CE (and ST) if they are found at the start of a continuation area, i came to a problem of patch [2/4] which i did not see when reviewing it yesterday: > + return grub_error (GRUB_ERR_BAD_FS, "invalid CE entry size"); It is not

Re: [PATCH 4/4] fs/iso9660: Incorrect check for entry boudary

2022-12-15 Thread Thomas Schmitt
Hi, On Wed, 14 Dec 2022 18:55:05 + Lidong Chen wrote: > [PATCH 4/4] fs/iso9660: Incorrect check for entry boudary s/boudary/boundary/ > An entry consists of the entry info and the component area. Component area is specific to the RRIP SL entry. So: s/An entry/An SL entry/ > The entry

Re: [PATCH 3/4] fs/iso9660: Avoid reading past the entry boundary

2022-12-15 Thread Thomas Schmitt
+ > data->susp_skip = entry->data[2]; >entry = (struct grub_iso9660_susp_entry *) ((char *) entry + > entry->len); > > -- > 2.35.1 > Reviewed-by: Thomas Schmitt But the expression (GRUB_ISO9660_SUSP_HEADER_SZ + 2) will need correction if my wish for #defi

Re: [PATCH 2/4] fs/iso9660: Prevent read past the end of system use area

2022-12-15 Thread Thomas Schmitt
Hi, On Wed, 14 Dec 2022 18:55:03 + Lidong Chen wrote: > In the code, the for loop advanced the entry pointer to the > next entry before checking if the next entry is within the > system use area boundary. Another issue in the code was that > there is no check for the size of system use area.

Re: [PATCH 1/4] fs/iso9660: Add check to prevent infinite loop

2022-12-15 Thread Thomas Schmitt
gt; + continue; > + } > + > if (node->have_dirents >= node->alloc_dirents) > { > struct grub_fshelp_node *new_node; > -- > 2.35.1 > Reviewed-by: Thomas Schmitt The second hunk will become very necessary when more

Re: [PATCH 0/4] fs/iso9660: Fix out-of-bounds read

2022-12-14 Thread Thomas Schmitt
Hi, i will review the patches hopefully tomorrow. But in [PATCH 2/4] and [4/4] i stumble over the statement that a SUSP entry has 5 bytes of size. This is not true. The minimum size is 4. In SUSP there are "Padding" PD (if its byte LEN_PD is 0) and "Stop" ST, which have 4 bytes. In RRIP there

Re: Possible memory fault in fs/iso9660 (correction)

2022-11-29 Thread Thomas Schmitt
Hi, i wrote: > > > I will think about creating such an ISO by help of xorriso and dd. Daniel Kiper wrote: > Yeah, that would be perfect... I believe to have created one. But grub-fstest does not produce a memory fault. See my mail Date: Tue, 29 Nov 2022 19:47:22 +0100 Message-Id:

Re: Possible memory fault in fs/iso9660 (correction)

2022-11-29 Thread Thomas Schmitt
Hi, Fengtao wrote: > I think: > (char *) entry < (char *) sua + sua_size - 3 && entry->len > 0 > is ok, or: > (char *) entry <= (char *) sua + sua_size - 4 && entry->len > 0 "4" would be overdone. There are SUSP and RRIP entries of length 4, which would be ignored if appearing at

Re: Possible memory fault in fs/iso9660 (correction)

2022-11-24 Thread Thomas Schmitt
Hi, (Again i Cc t.feng in the hope that the review is not finished yet. :)) Daniel Kiper wrote: > I am not an ISO format expert but your thinking LGTM. So you agree that "3" is really the right number if any remaining bytes fewer than 4 shall be ignored ? (I don't trust myself, although i made

Re: Possible memory fault in fs/iso9660 (correction)

2022-11-19 Thread Thomas Schmitt
Hi, i wrote: > I think the loop end condition should use 4 rather than 1: > (char *) entry < (char *) sua + sua_size - 4 && entry->len > 0 Urm ... better "3 rather than 1": (char *) entry < (char *) sua + sua_size - 3 && entry->len > 0 The memory fault by entry->len will appear if

Possible memory fault in fs/iso9660

2022-11-19 Thread Thomas Schmitt
Hi, (Cc-ing t.feng in the hope that this issue can become part of the code review.) While reviewing "[PATCH 7/9]" by t.feng, i wonder whether there is a bug in grub_iso9660_susp_iterate() in regard to the end of the SUSP data: for (entry = (struct grub_iso9660_susp_entry *) sua; (char *)

Re: [PATCH 7/9] fs/iso9660: Fix memory leak in grub_iso9660_susp_iterate

2022-11-19 Thread Thomas Schmitt
Hi, on Sat, 19 Nov 2022 18:39:44 +0800, t.feng via Grub-devel wrote: > Fix memory leak in grub_iso9660_susp_iterate. > > Fixes: 99373ce47(iso9660.c: Remove nested functions.) > > Signed-off-by: "t.feng" Reviewed-by: Thomas Schmitt H

Re: [PATCH 2/3] grub-mkrescue: Preserve a copy of the EFI bootloaders on the ISO9660 file system

2022-06-08 Thread Thomas Schmitt
9660 file > system. > > This is accomplished by removing the use of a temporary directory to create > the > efi/ content, to instead place it at the root of the ISO9660 content. > > Signed-off-by: Pete Batard Reviewed-by: Thomas Schmitt Tested-by: Thomas Schmitt I

Re: [PATCH 0/3] Add support for EFI file system transposition

2022-06-07 Thread Thomas Schmitt
Hi, Pete Batard wrote: > 2. It uses a efi.img to embed the UEFI bootloaders, but does not keep a copy > of these bootloaders on the ISO9660 file system itself, with the end result > that, when copying the media at the file system level, the '/efi/boot/' > directory and its content is missing. I

Re: [PATCH v2] tests: Refactor building xorriso command for iso9660 tests

2021-12-10 Thread Thomas Schmitt
Glenn Washburn > --- > Updates since v1: > * Reorder such that command line arguments are built in order, per Thomas' > suggestion > --- Reviewed-by: Thomas Schmitt Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel

Re: [PATCH] tests: Refactor building xorriso command for iso9660 tests

2021-12-08 Thread Thomas Schmitt
Hi, i think this change is beneficial for the maintainability of the test. But this sequence looks a bit confusing, albeit it is ok on the second glimpse: + XORRISO_ARGS="-as mkisofs $XORRISOFS_CHARSET -graft-points" + + if [ -z "${fs##*rockridge*}" ]; then +

Re: [PATCH 2/2] Make a "gdb" dprintf that tells us load addresses.

2021-10-20 Thread Thomas Schmitt
Hi, Robbie Harwood wrote: > dl.c:694: > add-symbol-file ... > dl.c:694: > add-symbol-file ... > dl.c:694: > add-symbol-file ... > > You can't easily copy-paste that into gdb. It needs to be processed to > remove the file:line stuff. How about hiding the "dl.c" lines

[PATCH v2] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons

2021-09-08 Thread Thomas Schmitt
r_r and -zisofs. Remove the -as mkisofs options which produce a Joliet filesystem tree. Signed-off-by: Thomas Schmitt --- tests/util/grub-fs-tester.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in index 4f581b638..a28e0

Re: [PATCH] [PATCH] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons

2021-09-08 Thread Thomas Schmitt
Hi, i am working on the requested changes. In my patch message i wrote: > > A bug > > in xorriso causes a distracting warning about FSLABEL being too long for > > Joliet. Shortcommings of Joliet cause warnings about symbolic links. Daniel Kiper wrote: > s/links./links too./? Rather not. Both

Re: [PATCH] [PATCH] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons

2021-09-08 Thread Thomas Schmitt
Hi, sorry for messing up the mail subject by "[PATCH] [PATCH]". (I added one of them already to the local commit message.) Shall i send new mail with better subject ? Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org

[PATCH] [PATCH] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons

2021-09-08 Thread Thomas Schmitt
iso9660" to the 32-byte FSLABEL list. Fix the xorriso run to produce compressed files which for now cause righteous failure of the test. Remove the option to produce a Joliet filesystem tree in order to concentrate on testing zisofs decompression. Signed-off-by: Thomas Schmitt --- tests/uti

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

2021-08-27 Thread Thomas Schmitt
Hi, i wrote: > > it might be contraproductive to set LANG=en_US.UTF-8. Glenn Washburn wrote: > My patch only sets LANG if empty, so in this case I don't think its > counterproductive, just not productive. Yes. It cautiously avoids to make the situation any worse. I meant my own announced plan

[PATCH] tests: Let xorriso fixely assume UTF-8 as local character set

2021-08-27 Thread Thomas Schmitt
CODESET. So override the result of nl_langinfo(CODESET) by options of xorriso -as mkisofs. Signed-off-by: Thomas Schmitt --- tests/util/grub-fs-tester.in | 22 +- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

2021-08-27 Thread Thomas Schmitt
Hi, it turns out that it might be contraproductive to set LANG=en_US.UTF-8. If the LANG locale is not listed by locale -a then nl_langinfo(3) returns "ANSI_X3.4-1968". I stumbled over this when testing LANG=de_DE.UTF8, which despite of my location is not an available locale on my machine. So

Re: [grub-fs-tester.in] zisofs test looks unsuitable

2021-08-26 Thread Thomas Schmitt
Hi. Glenn Washburn wrote: > I think the changes to get the test > working are worthy of inclusion so that the tests are ready when this > feature gets implemented. Do you think my draft of a commit message is ok ? Probably i should mention that this test will fail until zisofs is implemented.

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

2021-08-26 Thread Thomas Schmitt
Hi, i wrote: > > xorriso ... -as mkisofs -input-charset UTF-8 -output-charset UTF-8 Glenn Washburn wrote: > Do there need to be any UTF-8 locales installed (or any locales for > that matter) for this to work? My guess is no. I expect the same. It's not easy to test, though. In theory it is

Re: [grub-fs-tester.in] zisofs test looks unsuitable

2021-08-26 Thread Thomas Schmitt
s applied before any files had populated the emerging ISO image. So add "ziso9660" where other ISO 9660 tests get a shorter FSLABEL and re-arrange the commands of the xorriso run to bring -add and -zisofs before -set_filter_r. Signed-off-by: Thomas Schmitt diff --git a/tests/util/grub-fs-

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

2021-08-26 Thread Thomas Schmitt
Hi, now i know that i should have asked for the test-suite.log file. The problem turned out to be in Joliet, not in ISO 9660. Because Joliet gets its file names and other texts encoded as UCS-2 16-bit characters it is indeed inavoidable to define the meaning of the bytes in a Unix file name. The

Re: [grub-fs-tester.in] zisofs test looks unsuitable

2021-08-26 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > If I'm not mistaken, there is currently no test which actually > exercises that code, ie no test runs "grub-fs-tester ziso9660". I think > it could be a good idea to have one though. So we should get this to work. zisofs can be helpful to squeeze more operating system

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

2021-08-26 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > Despite having an alternative approach, do you have concerns or > objections to my patch? Not specifically. I initially only wanted to mention that there is a xorriso alternative to setting LANG. Both ways have different implications in detail. (LANG is global to the

[grub-fs-tester.in] zisofs test looks unsuitable

2021-08-25 Thread Thomas Schmitt
Hi, the xorriso run for testing zisofs in tests/util/grub-fs-tester.in looks not like it would cause any zisofs compression in the ISO. Line 1024: xorriso -compliance rec_mtime -set_filter_r --zisofs -- -zisofs default -as mkisofs ... some options ... --

Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test

2021-08-25 Thread Thomas Schmitt
Hi, Glenn Washburn wrote: > LANG must be set to something that supports international characters, > otherwise xorriso will refuse to include the file with name having > international characters, causing the test to fail. Can you tell me the exact error message from xorriso ? I have some

Re: grub-mkrescue fails with HFS+ error possibly due to directory size

2020-07-25 Thread Thomas Schmitt
Hi, what shall be done about the failure of grub-mkrescue for I386_EFI, X86_64_EFI, and POWERPC_IEEE1275 with very large input file trees ? https://lists.gnu.org/archive/html/grub-devel/2020-06/msg00062.html libisofs now emits a better error message Too many files in HFS+ directory tree

Re: grub-mkrescue fails with HFS+ error possibly due to directory size

2020-06-12 Thread Thomas Schmitt
Hi, (cc-ing bug-xorr...@gnu.org and the reporter of the problem.) I now have the Guix ISO which fails when created by grub-mkrescue with HFS+ tree. To my newest theory it is not about the number of files in a directory but about the total number of files in the tree and their name lengths. I

grub-mkrescue fails with HFS+ error possibly due to directory size

2020-06-11 Thread Thomas Schmitt
Hi, i just got a bug report about Guix ISO image production failing with libisofs: FAILURE : HFS+ map nodes aren't implemented The message stems from the HFS+ contribution to libisofs by Vladimir Serbinenko in 2012. About the only program which asks libisofs to produce HFS+ is grub-mkrescue,

Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...

2020-03-25 Thread Thomas Schmitt
Hi, Michael Chang wrote in patch 2/2: > ../../grub-core/fs/zfs/zfs.c:2263:44: error: array subscript '' is > outside the bounds of an interior zero-length array 'grub_uint16_t[0]' {aka > 'short unsigned int[0]'} [-Werror=zero-length-bounds] > [...] > The l_hash[0] is apparnetly an interior member

Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...

2020-03-24 Thread Thomas Schmitt
Hi, i wrote: > >(char *) _roles - (char *) sb > >+ grub_le_to_cpu32 (sb.dev_number) * sizeof(grub_uint16_t) PGNet Dev wrote: > grub-core/disk/mdraid1x_linux.c:183:6: error: cannot convert to a > pointer type My fault. I forgot the "&" before "sb". (char *)

Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...

2020-03-20 Thread Thomas Schmitt
Hi, Paul Menzel wrote: > 181 | (char *) _roles[grub_le_to_cpu32 (sb.dev_number)] >98 | grub_uint16_t dev_roles[]; /* Role in array, or 0x for a > 127 | struct grub_raid_super_1x sb; > [...] > Normally, it should be fixed by using `grub_uint16_t[]` instead of >

Re: grub-mkrescue: The blkid LABELs of its ISO 9660, HFS+, FAT filesystems

2020-01-17 Thread Thomas Schmitt
Hi, i wrote: > > [ ] Change HFS+ LABEL automatically by adding "_HFSPLUS" to the upto 32 > > characters of Volume Id. Daniel Kiper wrote: > Where this automatic change should happen? grub-mkrescue or libisofs? In libisofs. The decisive code gesture is in Vladimir's contribution

grub-mkrescue: The blkid LABELs of its ISO 9660, HFS+, FAT filesystems

2019-12-18 Thread Thomas Schmitt
Hi, during tests about the changes in blkid which are still discussed in https://github.com/karelzak/util-linux/pull/913 i noted that a grub-mkrescue ISO for x86 BIOS and EFI shows the same filesystem label with its ISO 9660 and HFS+ filesystem superblocks. # lsblk -o NAME,FSTYPE,LABEL

[PATCH] docs: workaround for grub-mkrescue with older MacBooks

2019-06-20 Thread Thomas Schmitt
Description of the workaround for firmware of older MacBooks which stalls with a grub-mkrescue ISO image for platform "x86_64-efi" on a USB stick. Signed-off-by: Thomas Schmitt --- diff --git a/docs/grub.texi b/docs/grub.texi index 308b250..a9b38e9 100644 --- a/docs/grub.texi

Problem to reply to an off-list mail of Daniel Kiper

2019-06-15 Thread Thomas Schmitt
Hi, @Daniel Kiper: I am trying to answer your mail from yesterday. (Offlist) Re: grub-mkrescue: Problem with MBR partition table at start of EFI partition But my mail provider's server forwards to me this rejection: dki...@net-space.pl: SMTP error from remote server for MAIL FROM

Re: Several GNU projects wondering about the reason for mformat partition table

2019-06-02 Thread Thomas Schmitt
Hi, thanks a lot for the background info. Alain Knaff wrote: > A platform that expects the disk to be unpartitioned would just ignore > this mini partition table. The EFI platform encounters the mformat-produced image in a partition. Nearly all implementations indeed ignore the MBR-ish table in

  1   2   3   >