[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Possible memory fault in fs/iso9660 (correction)
From: |
Thomas Schmitt |
Subject: |
Re: Possible memory fault in fs/iso9660 (correction) |
Date: |
Tue, 29 Nov 2022 19:47:22 +0100 |
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