Hi,
Sorry for top posting...
I have just realized colleague of mine is doing some work in the ISO 9660
code including part which we are discussing here. I asked her to post the
patches on the grub-devel. You can expect them soon. Please take a look
at them and comment/review.
Daniel
On Tue, Nov 29, 2022 at 07:47:22PM +0100, Thomas Schmitt wrote:
> 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 the end of the SUSP controlled area.
>
>
> > I am also not familiar with ISO format.
>
> I seem to be the last active programmer on that topic.
>
> As stated on 24 Nov, i see other potential memory faults in the code of
> grub-core/fs/iso9660.c if the ISO image contains incorrect SUSP entries.
>
> ---------------------------------------------------------------------------
>
> I began to hack an ISO image so that it shows non-SUSP data after the
> SUSP data. (See below.)
>
> But now i am having noob problems with grub-fstest.
>
> I pulled the source from git://git.savannah.gnu.org/grub on Debian 11 and
> built it as prescribed in INSTALL.
> Nevertheless grub-fstest does not show a memory fault with:
>
> valgrind ./grub-fstest /dvdbuffer/grub_test_non_susp.iso ls /
>
> gdb says that the execution enters grub_iso9660_susp_iterate()
> Breakpoint 1, 0x000055555557dde4 in grub_iso9660_susp_iterate ()
> but gives no further information, because
> (No debugging symbols found in ./grub-fstest)
>
> Next i tried to insert visible messages into grub_iso9660_susp_iterate():
> grub_error (GRUB_ERR_BAD_NUMBER, "grub_iso9660_susp_iterate: called");
> ...
> grub_error (GRUB_ERR_BAD_NUMBER, "grub_iso9660_susp_iterate: before loop");
> ...
> grub_error (GRUB_ERR_BAD_NUMBER,
> "grub_iso9660_susp_iterate: sua + sua_size - entry = %ld",
> (long int) ((char *) sua + sua_size - (char *) entry));
> I do not see any of them when running with above arguments.
>
> So how can i make grub-core/fs/iso9660.c debuggable or let it emit visible
> messages ?
>
> ---------------------------------------------------------------------------
> The test ISO is made by these commands in bash:
>
> # Create an ISO with a playground of SUSP data.
> echo dummy >dummy_file
> test -e grub_test_non_susp.iso && rm grub_test_non_susp.iso
> xorriso \
> -xattr on \
> -outdev grub_test_non_susp.iso \
> -map dummy_file /dummy_file \
> -setfattr user.x y /dummy_file -- \
> -padding 0
>
> # Search for the AL entry of length 0x0c which holds attribute user.x.
> # (AL is the entry type of my invention AAIP. It gets ignored by all
> # readers except xorriso. So it is a good playground for manipulations.)
> # (There is also another AL entry of size 0x10 in the CE area of the root
> # directory.)
> al=$(grep -a -o -b 'AL'$'\x0c'$'\x01' grub_test_non_susp.iso | \
> sed -e 's/:/ /' | awk '{print $1}')
>
> # Replace length field value 0x0c by 0x0a.
> # This leaves the last 2 bytes of the AL entry as trailing non-SUSP data
> # in the System Use field of the directory entry.
> al_length_offset=$(expr $al + 2)
> echo $'\x0a' | \
> dd of=grub_test_non_susp.iso \
> bs=1 count=1 seek="$al_length_offset" conv=notrunc
>
> Inspection by a hex dumper shows that the AL entry indeed announces the
> desired (short) length of 10.
>
> ---------------------------------------------------------------------------
>
> I also learned by doing and then by reading specs that a padding byte after
> the System Use data is present if needed to get an even number of bytes
> as size of the directory record.
>
> This could explain the existing "- 1" in GRUB's code.
>
> Above ISO is supposed to show 3 non-SUSP bytes at the end of the System Use
> field: 2 from my dd manipulation, 1 as regular padding.
>
> Have a nice day :)
>
> Thomas
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
