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

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. For a corrupted system, the size of system use area can be less than

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

This is the v2 patches set which addressed the review comments from Thomas Schmitt. Many thanks to Thomas for the review comments as well as the detailed explanation and test instruction. Patch 0005 is a new patch addressing an old bug pointed out by Thomas. Thanks Thomas for providing the fix.

[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 Ch

[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-by:

[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 to

[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 65c8862b6

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

Hi, On Wed, 18 Jan 2023 08:23:54 + 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 while loop, thus > caused infinite loop. The fix added a check for the > end of the block and e

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

Hi, On Wed, 18 Jan 2023 08:23:55 + 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 v2 3/5] fs/iso9660: Avoid reading past the entry boundary

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 insertions(+), 2 deletions(-) > >

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

Hi, On Wed, 18 Jan 2023 08:23:57 + Lidong Chen wrote: > 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

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

Hi, On Wed, 18 Jan 2023 08:23:58 + 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 > interpretation. In case of CE this would lead to premature end of > processing the SUSP entries of

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

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 in

Re: [PATCH v1 1/2] Skip diskfilter on powerpc CDs

"Vladimir 'phcoder' Serbinenko" writes: > This is not properly restricted to just raid. Only mdraid 1.1 is > affected. The condition should probably be along the lines "if msdos > is embedded in a device of unknown size, then skip mdraid 1.1 probing" My testing suggested it was both mdraid1x and

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 + 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

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 + 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 >> interpretation. In case of CE t

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 + 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 while loop, thus >> caused