[PATCH 1/2] iso9660: Fix Forth return value for iso9660_files_open()

[Andreas Färber]

There's a mixup of 0 and -1. Don't return failure if iso9660_open() succeeded. Don't return success in case
iso9660_open() or iso9660_mount() failed.

This resolves both a hang when trying to boot cd:,\:tbxi and failure to boot either cd:,\ppc\bootinfo.txt
or cd:,\ppc\chrp\bootfile.exe despite present on the AIX 6.1 disk.

Cc: Mark Cave-Ayland 
Signed-off-by: Andreas Färber 
---
 fs/iso9660/iso9660_fs.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/iso9660/iso9660_fs.c b/fs/iso9660/iso9660_fs.c
index 5714d6d..3fb5c16 100644
--- a/fs/iso9660/iso9660_fs.c
+++ b/fs/iso9660/iso9660_fs.c
@@ -49,7 +49,7 @@ iso9660_files_open( iso9660_info_t *mi )
    if ( mi->volume == NULL ) {
    	free( path );
    	close_io( fd );
-		RET( -1 );
+		RET( 0 );
    }

mi->common->dir = iso9660_opendir( mi->volume, path );
@@ -59,11 +59,11 @@ iso9660_files_open( iso9660_info_t *mi )
    		iso9660_umount( mi->volume );
    		close_io( fd );
    		free( path );
-			RET( -1 );
+			RET( 0 );
    	}
    	mi->common->type = FILE;
    	free( path );
-		RET( 0 );
+		RET( -1 );
    }
    mi->common->type = DIR;
    free( path );

[PATCH 2/2] iso9660: Make name comparison case-insensitive

[Andreas Färber]

Make seek_name() compare the path component in a case-insensitive way.
Should fix an issue reported by Mark.

Cc: Mark Cave-Ayland 
Cc: Laurent Vivier 
Signed-off-by: Andreas Färber 
---
 fs/iso9660/iso9660_opendir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/iso9660/iso9660_opendir.c b/fs/iso9660/iso9660_opendir.c
index 10cb89d..49eab4b 100644
--- a/fs/iso9660/iso9660_opendir.c
+++ b/fs/iso9660/iso9660_opendir.c
@@ -55,7 +55,7 @@ static struct iso_directory_record * seek_name(iso9660_VOLUME *volume,
    while ((idr = iso9660_readdir(dir)) != NULL)
    {
    	iso9660_name(volume, idr, name_buf);
-		if (strcmp(name, name_buf) == 0)
+		if (strcasecmp(name, name_buf) == 0)
    	{
    		result = idr_new(idr);
    		iso9660_closedir(dir);

[Blue Swirl]

On Sun, Sep 26, 2010 at 9:01 PM, Andreas Färber  wrote:
> There's a mixup of 0 and -1. Don't return failure if iso9660_open() succeeded. Don't return success in case
> iso9660_open() or iso9660_mount() failed.
>
> This resolves both a hang when trying to boot cd:,\:tbxi and failure to boot either cd:,\ppc\bootinfo.txt
> or cd:,\ppc\chrp\bootfile.exe despite present on the AIX 6.1 disk.

There are still some other problems with iso9660. If I apply these
patches and change Sparc32 to use iso9660 instead of grubfs iso9660,
SILO crashes for example with Aurora 1.0:
Jumping to entry point 00004000 for type 00000005...
switching to new context:
SILO Unhandled Exception 0x00000007
PC = 0xffd02a28 NPC = 0xffd02a2c
Stopping execution

This is unaligned access exception. It seems to happen because ofmem
structure gets corrupted, maybe by free() or malloc() call after
something (iso9660?) has clobbered memory slightly outside of the
allocated area.

[Blue Swirl]

Thanks, applied both.

On Mon, Sep 27, 2010 at 8:38 PM, Blue Swirl  wrote:
> On Sun, Sep 26, 2010 at 9:01 PM, Andreas Färber  wrote:
> > There's a mixup of 0 and -1. Don't return failure if iso9660_open() succeeded. Don't return success in case
> > iso9660_open() or iso9660_mount() failed.
> >
> > This resolves both a hang when trying to boot cd:,\:tbxi and failure to boot either cd:,\ppc\bootinfo.txt
> > or cd:,\ppc\chrp\bootfile.exe despite present on the AIX 6.1 disk.
>
> There are still some other problems with iso9660. If I apply these
> patches and change Sparc32 to use iso9660 instead of grubfs iso9660,
> SILO crashes for example with Aurora 1.0:
> Jumping to entry point 00004000 for type 00000005...
> switching to new context:
> SILO Unhandled Exception 0x00000007
> PC = 0xffd02a28 NPC = 0xffd02a2c
> Stopping execution
>
> This is unaligned access exception. It seems to happen because ofmem
> structure gets corrupted, maybe by free() or malloc() call after
> something (iso9660?) has clobbered memory slightly outside of the
> allocated area.

[PATCH 1/2] iso9660: Fix Forth return value for iso9660_files_open()

[Mark Cave-Ayland]

Blue Swirl wrote:
> Thanks, applied both.

Excellent work! Thanks for managing to sort this out Andreas - these 
have been sat in my inbox waiting for me to review, but I just haven't 
managed to find a spare moment. They both look good though.

ATB,

Mark.

[Blue Swirl]

On Mon, Sep 27, 2010 at 8:38 PM, Blue Swirl  wrote:
> On Sun, Sep 26, 2010 at 9:01 PM, Andreas Färber  wrote:
> > There's a mixup of 0 and -1. Don't return failure if iso9660_open() succeeded. Don't return success in case
> > iso9660_open() or iso9660_mount() failed.
> >
> > This resolves both a hang when trying to boot cd:,\:tbxi and failure to boot either cd:,\ppc\bootinfo.txt
> > or cd:,\ppc\chrp\bootfile.exe despite present on the AIX 6.1 disk.
>
> There are still some other problems with iso9660. If I apply these
> patches and change Sparc32 to use iso9660 instead of grubfs iso9660,
> SILO crashes for example with Aurora 1.0:
> Jumping to entry point 00004000 for type 00000005...
> switching to new context:
> SILO Unhandled Exception 0x00000007
> PC = 0xffd02a28 NPC = 0xffd02a2c
> Stopping execution
>
> This is unaligned access exception. It seems to happen because ofmem
> structure gets corrupted, maybe by free() or malloc() call after
> something (iso9660?) has clobbered memory slightly outside of the
> allocated area.

Since r872  I can use iso9660 without a crash on Sparc32. One issue
was that there were no problems when using nographic, but with
graphical console the crashes would happen. Avoiding a lot of
malloc/free traffic when writing to screen seems to help, but the
underlying bug that corrupts ofmem probably still lurks somewhere.

[PATCH 1/2] iso9660: Fix Forth return value for iso9660_files_open()

[Mark Cave-Ayland]

Blue Swirl wrote:
> Since r872  I can use iso9660 without a crash on Sparc32. One issue
> was that there were no problems when using nographic, but with
> graphical console the crashes would happen. Avoiding a lot of
> malloc/free traffic when writing to screen seems to help, but the
> underlying bug that corrupts ofmem probably still lurks somewhere.

Ah that's good to know :)  Although I don't think it's (just?) an ofmem 
bug, because SPARC32 doesn't use ofmem yet - we're still waiting for you 
to do the conversion! ;D

ATB,

Mark.

[Blue Swirl]

On Thu, Sep 30, 2010 at 9:50 AM, Mark Cave-Ayland
 wrote:
> Blue Swirl wrote:
> > Since r872  I can use iso9660 without a crash on Sparc32. One issue
> > was that there were no problems when using nographic, but with
> > graphical console the crashes would happen. Avoiding a lot of
> > malloc/free traffic when writing to screen seems to help, but the
> > underlying bug that corrupts ofmem probably still lurks somewhere.
>
> Ah that's good to know :)  Although I don't think it's (just?) an ofmem bug,
> because SPARC32 doesn't use ofmem yet - we're still waiting for you to do
> the conversion! ;D

I meant the struct ofmem in arch/sparc32/lib.c, the fields get strange
values. Calling free() with memory that was not allocated by malloc()
or with a pointer to the middle of the block could be suspect, but I
don't see any of those.