[hatari-devel] Hatari crashes with Mental Hangover demo

[Eero Tamminen]

Hi,

On lauantai 30 huhtikuu 2011, Nicolas Pomarède wrote:
> But in fact I think we should have some kind of "DMA_memcpy" that would
> be used from FDC / HDC when we want to copy memory from Hatari to the
> emulated RAM. This function would check each written byte is in ST RAM
> or I/O space and would mask addresses to 24 bits.

I added stMemory_SafeCopy() function.


> This way, we would have a common function to correctly copy data from
> disk images to the emulated RAM or IO space.

Half of the potentially unsafe operations are fread()s & fwrite()s to STRam,
not just memcpy()s...  See e.g. gemdos.c which I've fixed earlier or the new
hdc.c changes.


> In that case, ignoring is fine, because it's not the cause of the
> problem. But as above, it would be more correct to not ignore the
> request but ensure each destination address is corrrecly handled to stay
> in the RAM / ROM / IO space of the emulated machine (even if writing in
> ROM should have no effect, it should not be forbidden)

The check in stMemory.h is:
-------------
static inline bool STMemory_ValidArea(Uint32 addr, int size)
{
        if (size >= 0 && addr+size < 0xff0000 &&
            (addr+size < STRamEnd || addr >= 0xe00000))
        {
                return true;
        }
        return false;
}
-------------

stMemory_SafeCopy() copies only bytes that above states to be valid, it
ignores bytes going to invalid addresses. (As I don't see what it e.g.
should do to bytes between STRamEnd and 0xe00000.)


>>>> PS. While doing the patches, I noticed:
...
>>  From above you can see that "HDCSectorCount" can never get any other
>>  value
>> than zero (unlike e.g. FDCSectorCountRegister which seems to be used
>> for similar purpose).  So what's the point in having that variable?
>> 
>> hdc.c code gets the HDC sector count with this macro, it doesn't use
>> the variable:
>> #define HD_SECTORCOUNT(a) (a.command[4]&  0xFF)        /* get sector
>> count */

To me it seems that using HDCSectorCount is fairly obviously wrong. As far
as I see, the correct value comes from HD_SECTORCOUNT() which isn't
used by fdc.c...

Thomas?


	- Eero