[PATCH] disk/mdraid1x_linux: Prevent infinite recursion

e fix adds checks for both the superblock region and the data region when parsing the superblock metadata in grub_mdraid_detect(). Signed-off-by: Lidong Chen --- grub-core/disk/mdraid1x_linux.c | 78 + 1 file changed, 78 insertions(+) diff --git a/grub-core/disk/md

[PATCH] disk/mdraid1x_linux: Prevent infinite recursion

e fix adds checks for both the superblock region and the data region when parsing the superblock metadata in grub_mdraid_detect(). Signed-off-by: Lidong Chen --- grub-core/disk/mdraid1x_linux.c | 66 + 1 file changed, 66 insertions(+) diff --git a/grub-core/disk/md

Re: [PATCH v3] Fix XFS directory extent parsing

On Sep 27, 2023, at 5:43 PM, Jon DeVree wrote: The XFS directory entry parsing code has never been completely correct for extent based directories. The parser correctly handles the case where the directory is contained in a single extent, but then mistakenly assumes the data blocks for the

[PATCH 1/1] fs/xfs: Incorrect short form directory data boundary check

pointer can cause a failure. The fix is to include the boundary check into the 'for' loop condition. Signed-off-by: Lidong Chen --- grub-core/fs/xfs.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c index b91cd32b4..ebf962793 100644

[PATCH v2 0/1] fs/udf: Fix out of bounds access

Thanks Daniel and Darren for reviewing the changes. This v2 addressed Darren's comments. Lidong Chen (1): fs/udf: Fix out of bounds access grub-core/fs/udf.c | 38 ++ 1 file changed, 38 insertions(+) -- 2.39.1

[PATCH v2 1/1] fs/udf: Fix out of bounds access

Implemented a boundary check before advancing the allocation descriptors pointer. Signed-off-by: Lidong Chen Reviewed-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/fs/udf.c | 38 ++ 1 file changed, 38 insertions(+) diff --git a/grub-core/fs

Re: [PATCH 1/1] fs/udf: Fix out of bounds access

On Jun 2, 2023, at 2:43 AM, Darren Kenny wrote: Hi Li, In general looks good... On Thursday, 2023-06-01 at 18:50:19 UTC, Lidong Chen wrote: Implemented a boundary check before advancing the allocation descriptors pointer. Signed-off-by: Lidong Chen --- grub-core/fs/udf.c | 36

[PATCH v2 1/1] xfs: Fix issues found while fuzzing the XFS filesystem

Harwood Signed-off-by: Marta Lewandowska Signed-off-by: Lidong Chen --- grub-core/fs/xfs.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c index d6de7f1a2..72051eb5e 100644 --- a/grub-core/fs/xfs.c +++ b/grub-core/fs/xfs.c

[PATCH 1/1] fs/udf: Fix out of bounds access

Implemented a boundary check before advancing the allocation descriptors pointer. Signed-off-by: Lidong Chen --- grub-core/fs/udf.c | 36 1 file changed, 36 insertions(+) diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c index 12e88ab62..2359222eb 100644

[PATCH 1/1] xfs: Fix issues found while fuzzing the XFS filesystem

Harwood Signed-off-by: Marta Lewandowska Signed-off-by: Lidong Chen --- grub-core/fs/xfs.c | 32 1 file changed, 32 insertions(+) diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c index d6de7f1a2..a0aaa3aa8 100644 --- a/grub-core/fs/xfs.c +++ b/grub-core/fs

[PATCH v2 3/4] fs/hfsplus: Set grub errno to prevent NULL pointer access

When an invalid node size is detected in grub_hfsplus_mount(), data pinter is freed. Thus, file->data is not set. The code should also set the grub error when that happens to indicate an error and to avoid accessing the unintialized file->data in grub_file_close(). Signed-off-by: Lidon

[PATCH v2 1/4] fs/hfsplus: Validate btree node size

The invalid btree node size can cause crashes when parsing the btree. The fix is to ensure the btree node size is within the valid range defined in the HFS Plus techical note, TN1150. https://developer.apple.com/library/archive/technotes/tn/tn1150.html Signed-off-by: Lidong Chen --- grub-core

[PATCH v2 4/4] fs/hfsplus: Mark error strings for translation

Signed-off-by: Lidong Chen --- grub-core/fs/hfsplus.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index cf13e8a63..793cbdba2 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c @@ -277,7 +277,7

[PATCH v2 0/4] Prevent out-of-bound reads

v2: - patch 4/4: Only mark the informative errors for translation based on the comments. - patch 2/4: Include a reference to the fix in the patch 2/4 commit message. - No changes to the rest of 2 patches. Lidong Chen (4): fs/hfsplus: Validate btree node size fs/hfsplus: Prevent out of bound

[PATCH v2 2/4] fs/hfsplus: Prevent out of bound access in catalog file

/tn1150.html Signed-off-by: Lidong Chen --- grub-core/fs/hfsplus.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index 1ffebc8be..9c1f12574 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c @@ -87,6 +87,9 @@ struct

Re: [PATCH 3/4] fs/hfsplus: Set grub errno to prevent NULL pointer access

On Apr 20, 2023, at 3:07 PM, Vladimir 'phcoder' Serbinenko wrote: On Thu, Apr 20, 2023 at 8:00 PM Lidong Chen mailto:lidong.c...@oracle.com>> wrote: When an invalid node size is detected in grub_hfsplus_mount(), data pinter is freed. Thus, file->data is not set. The code should

Re: [PATCH 2/4] fs/hfsplus: Prevent out of bound access in catalog file

only the number of bytes required to hold the name. The keyLength field determines the actual length of the key; it varies between kHFSPlusCatalogKeyMinimumLength (6) to kHFSPlusCatalogKeyMaximumLength (516).” Regards, Lidong On Apr 20, 2023, at 10:59 AM, Lidong Chen wrote: A corrupted hfsplus can

Re: [PATCH 4/4] fs/hfsplus: Mark error strings for translation

> This patch has been suggested by me because I thought the lack of N_() > was a mistake. Your comments shed some light to this. Though I still > think some messages, e.g. "not a HFS+ filesystem", could be translated. I can revert the changes, except the general messages as menti

[PATCH 2/4] fs/hfsplus: Prevent out of bound access in catalog file

A corrupted hfsplus can have a catalog key that is out of range. This can lead to out of bound access when advancing the pointer to access catalog file info. Signed-off-by: Lidong Chen --- grub-core/fs/hfsplus.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/grub-core/fs

[PATCH 1/4] fs/hfsplus: Validate btree node size

The invalid btree node size can cause crashes when parsing the btree. The fix is to ensure the btree node size is within the valid range defined in the HFS Plus techical note, TN1150. https://developer.apple.com/library/archive/technotes/tn/tn1150.html Signed-off-by: Lidong Chen --- grub-core

[PATCH 4/4] fs/hfsplus: Mark error strings for translation

Signed-off-by: Lidong Chen --- grub-core/fs/hfsplus.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index cf13e8a63..f91af1477 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c

[PATCH 0/4] Prevent out-of-bound reads

This set of patches adds checks to ensure the node size is valid before accessing it. In addition, error messages are marked for translation. Lidong Chen (4): fs/hfsplus: Validate btree node size fs/hfsplus: Prevent out of bound access in catalog file fs/hfsplus: Set grub errno to prevent

[PATCH 3/4] fs/hfsplus: Set grub errno to prevent NULL pointer access

When an invalid node size is detected in grub_hfsplus_mount(), data pinter is freed. Thus, file->data is not set. The code should also set the grub error when that happens to indicate an error and to avoid accessing the unintialized file->data in grub_file_close(). Signed-off-by: Lidon

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

Tested-by: Lidong Chen mailto:lidong.c...@oracle.com>> On Mar 8, 2023, at 10:28 PM, Lidong Chen wrote: Hi, Thanks for the detailed instruction for running the test! The patches look good to me. I ran the tests with and without the patches, I got the expected result. Thanks, Lidong

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

Hi, Thanks for the detailed instruction for running the test! The patches look good to me. I ran the tests with and without the patches, I got the expected result. Thanks, Lidong > On Mar 7, 2023, at 8:56 AM, Thomas Schmitt wrote: > > Hi, > > SUSP 1.12 says: > > The "CE" System Use

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

> On Feb 2, 2023, at 11:35 AM, Daniel Kiper wrote: > > On Fri, Jan 20, 2023 at 07:39:38PM +0000, Lidong Chen wrote: >> There is no check for the end of block when reading >> directory extents. It resulted in read_node() always >> read from the same offset in the

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

than the size of minimum SUSP entry size (4 bytes). These can cause buffer overrun. The fixes added the checks to ensure the read is valid and within the boundary. Signed-off-by: Lidong Chen Reviewed-by: Thomas Schmitt --- grub-core/fs/iso9660.c | 30 +++--- 1 file changed

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

An SL entry consists of the entry info and the component area. The entry info should take up 5 bytes instead of sizeof (*entry). The area after the first 5 bytes is the component area. It is incorrect to use the sizeof (*entry) to check the entry boundary. Signed-off-by: Lidong Chen Reviewed

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

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 insertions(+), 2 deletions(-) diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c index

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

There is no check for the end of block when reading directory extents. It resulted in read_node() always read from the same offset in the while loop, thus caused infinite loop. The fix added a check for the end of the block and ensure the read is within directory boundary. Signed-off-by: Lidong

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

This v3 patches set addressed v2 review comments for patch 5. There is no changes to patch 1 to 4. Tested patch 5, the fixes were able to detect the endless loop, and Grub exited accordingly. Lidong Chen (5): fs/iso9660: Add check to prevent infinite loop fs/iso9660: Prevent read past

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

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(+) diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c index ca45b3424..3ddb06ed4 100644 --- a/grub-core/fs/iso9660.c

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

On Jan 20, 2023, at 3:49 AM, Thomas Schmitt mailto:scdbac...@gmx.net>> wrote: 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 o

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

limit but set the default to 31. Lidong Chen wrote: I am not sure I understand the hard limit vs the default in terms of checking the number of hops. xorriso produces and reads ISO 9660 filesystems. At production time, the limit will be adjustable. The default will be 31 to prevent files which

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

> On Jan 18, 2023, at 8:07 AM, Thomas Schmitt wrote: > > Hi, > > On Wed, 18 Jan 2023 08:23:54 +0000 Lidong Chen wrote: >> There is no check for the end of block when reading >> directory extents. It resulted in read_node() always >> read from the same offset

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

> On Jan 18, 2023, at 8:21 AM, Thomas Schmitt wrote: > > Hi, > > On Wed, 18 Jan 2023 08:23:58 +0000 Lidong Chen wrote: >> 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 >&

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

Hi, > On Jan 18, 2023, at 8:31 AM, Thomas Schmitt wrote: > > Hi, > > On Wed, 18 Jan 2023 08:23:53 +0000 Lidong Chen wrote: >> Thomas also pointed out the issue of the potential endless >> loops by CE. Since the sugguested fix requires a bit more >> investi

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

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 the file. In case of ST this could cause following non-SUSP bytes

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

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 insertions(+), 2 deletions(-) diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c index

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

An SL entry consists of the entry info and the component area. The entry info should take up 5 bytes instead of sizeof (*entry). The area after the first 5 bytes is the component area. It is incorrect to use the sizeof (*entry) to check the entry boundary. Signed-off-by: Lidong Chen Reviewed

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

than the size of minimum SUSP entry size (4 bytes). These can cause buffer overrun. The fixes added the checks to ensure the read is valid and within the boundary. Signed-off-by: Lidong Chen --- grub-core/fs/iso9660.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions

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

test (ran for 2 days) confirmed that the patches fixed the issues. Lidong Chen (5): fs/iso9660: Add check to prevent infinite loop fs/iso9660: Prevent read past the end of system use area fs/iso9660: Avoid reading past the entry boundary fs/iso9660: Incorrect check for entry boundary fs

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

There is no check for the end of block when reading directory extents. It resulted in read_node() always read from the same offset in the while loop, thus caused infinite loop. The fix added a check for the end of the block and ensure the read is within directory boundary. Signed-off-by: Lidong

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

Hi Thomas, > On Jan 11, 2023, at 3:54 AM, Thomas Schmitt wrote: > > 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: > >

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

uot;, 2) == 0) >>> + { >>> + ce = (struct grub_iso9660_susp_ce *) entry; >>> + if (ce_block >>> + != grub_le_to_cpu32 (ce->blk) << GRUB_ISO9660_LOG2_BLKSZ >>> + || off != grub_le_to_cpu

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

On Dec 16, 2022, at 4:57 AM, Thomas Schmitt mailto:scdbac...@gmx.net>> wrote: Hi, i realize that my previous proposal opens a possibility for regression with a very bad ISO image. The danger is in an endless loop by a CE entry which points to itself. The bug which i want to see fixed

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

> On Dec 16, 2022, at 4:57 AM, Thomas Schmitt wrote: > > Hi, > > i realize that my previous proposal opens a possibility for regression with > a very bad ISO image. > The danger is in an endless loop by a CE entry which points to itself. > The bug which i want to see fixed currently prevents

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

> On Dec 15, 2022, at 10:20 AM, Thomas Schmitt wrote: > > Hi, > > On Wed, 14 Dec 2022 18:55:05 +0000 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

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

> On Dec 15, 2022, at 10:08 AM, Thomas Schmitt wrote: > > Hi, > > On Wed, 14 Dec 2022 18:55:04 +0000 Lidong Chen wrote: >> Added a check for the SP entry data boundary before reading it. >> >> Signed-off-by: Lidong Chen >> --- >> grub-core

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

> On Dec 15, 2022, at 10:00 AM, Thomas Schmitt wrote: > > Hi, > > On Wed, 14 Dec 2022 18:55:03 +0000 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

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

> On Dec 15, 2022, at 9:52 AM, Thomas Schmitt wrote: > > Hi, > > On Wed, 14 Dec 2022 18:55:02 +0000 Lidong Chen wrote: >> There is no check for the end of block When reading > > s/When/when/ > >> directory extents. It resulted in read_node() always >&

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

> On Dec 14, 2022, at 1:42 PM, Thomas Schmitt wrote: > > 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

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

There is no check for the end of block When reading directory extents. It resulted in read_node() always read from the same offset in the while loop, thus caused infinite loop. The fix added a check for the end of the block and ensure the read is within directory boundary. Signed-off-by: Lidong

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

This patches set fix a few out-of-bound reads and an infinite loop in fs/iso9660. The main issues are that there is no validation for the SUSP/RRIP entry size and no check for the boundary before read. Lidong Chen (4): fs/iso9660: Add check to prevent infinite loop fs/iso9660: Prevent read

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

than the size of SUSP entry size (5 bytes). These can cause buffer overrun. The fixes added the checks to ensure the read is valid and within the boundary. Signed-off-by: Lidong Chen --- grub-core/fs/iso9660.c | 31 +++ 1 file changed, 27 insertions(+), 4 deletions

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

record. Added a check for for the component length before reading the component record. Signed-off-by: Lidong Chen --- grub-core/fs/iso9660.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c index 67aa8451c

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

Added a check for the SP entry data boundary before reading it. Signed-off-by: Lidong Chen --- grub-core/fs/iso9660.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c index 9170fa820..67aa8451c 100644