Re: [hatari-devel] Crash with HD IDE->ACSI image change?

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On sunnuntai 12 lokakuu 2014, Thomas Huth wrote:
> schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
> > With your fix, DESKTOP.INF & NEWDESK.INF could get written to
> > user's host root directory (if/when Hatari has access rights
> > to write there).
> 
> How that?

Sorry, that was a brainfart, I somehow noticed only your gemdos.c
modification, not the vdi.c one (and though that gemdos.c was all
the "fix").

Issue is not your modification, but an already existing bug. I meant
that GemDOS_CreateHardDriveFileName() can return without modifying
the given string buffer, and it has no return value to indicate that
failure.

In VDI_FixDesktopInf(), and some other cases, those buffers aren't
initialized beforehand.  They contain whatever content the given
memory area had previously contained (and zeroes if the area returned
by malloc hadn't been used previously in the process life-cycle).

After a while, it's quite possible that this memory area happens
to contain a "valid" filename.

Other ways to fix this besides having full drive check in caller,
and assert in gemdos function, are:
- zeroing destination string before error exit (dest[0] = 0), and/or
- returning error in this case so that caller can bail out


> > I.e. it's this part that should be fixed, not gemdos.c:
....
> That's one of the spots that I just fixed. Which modification do you
> propose?

I think it would be cleaner if code for checking GEMDOS emulation
for specific drive would be in gemdos.c, and just called from
vdi.c.


> > Besides checking for GEMDOS HD emulation, it probably
> > should also output a warning if GEMDOS HD writes are
> > disabled with ConfigureParams.HardDisk.nWriteProtection.
> 
> Feel free to add that check.

Ok.


	- Eero