Re: [hatari-devel] Disk driver memory access checks (was: Cycle exact setting affects nf_scsidrv)

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Hi,

On 10/13/18 10:15 AM, Uwe Seimet wrote:
Please see the attachment. This is the result in case of an invalid RAM
address when reading: >
scsidrv_inout: handle=0, dir=0, cmd_len=10, buffer=0x5632a9f5c9e0,
                transfer_len=65536, sense_buffer=0x5632a91dbfb0, timeout=10000,
                cmd=$28:$00:$00:$00:$00:$00:$00:$00:$80:$00ERROR: pBank flags mismatch: 0x2 & 0x1 (RAM = 0x1)
WARN : scsidrv_interface_fetures: Invalid RAM range 0xe00000+65536
  -> -1

Thanks!


Note that writing is allowed, it does not have to be a RAM address in
this case.

If you mean that write could also be from ROM area, you need to use:
	ABFLAG_RAM | ABFLAG_ROM

(see what the other disk drivers do.)

New patch?


	- Eero

Ps. Thomas, I didn't see Hatari IDE driver doing these checks, but on
quick check, I didn't find it doing any buffer transfers, only
byte/word/long register reads & writes...?

Best regards

Uwe

Hi,

On 10/12/18 12:25 AM, Uwe Seimet wrote:
Is st_buffer data something that could be program code read from
the SCSI disk?  If yes, all caches would need to be flushed for that.

Yes, you are probably right. Just checked that I'm also flushing both in
HDDRIVER. One more updated patch attached. The sense data buffer and
the buffer for the bus name do not need flushing the instruction cache
because you cannot transfer program code this way.

Thanks, I pushed you patch:
https://hg.tuxfamily.org/mercurialroot/hatari/hatari/rev/d6355bb2dc25

When checking that, I noticed that nf_scsidrv.c doesn't validate
that the memory addresses specified through the NF API are valid,
like GEMDOS HD driver does:
-----------------------------
          /* Check that read is to valid memory area */
          if ( !STMemory_CheckAreaType ( Addr, Size, ABFLAG_RAM ) )
          {
                  Log_Printf(LOG_WARN, "GEMDOS Fread() failed due to
invalid RAM range 0x%x+%i\n", Addr, Size);
                  Regs[REG_D0] = GEMDOS_ERANGE;
                  return true;
          }
-----------------------------

Could you send also a (tested) patch that adds those checks?


	- Eero





Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/