gamozolabs / applepie

A hypervisor for fuzzing built with WHVP and Bochs
366 stars 56 forks source link

Bochs: 8GB ram #32

Closed thebabush closed 5 years ago

thebabush commented 5 years ago

I did this for a project of mine and it seems to work (lol). Bochs uses Bit32u only for block indexes and to count used_blocks (if I'm correct) so it should be fine. Blocks are 4MB-long so that should leave enough bits to keep the Bit32u working (4GB ram overflows the Bit32u cast in the memory allocation and that seems to be the only issue in increasing the mem size). I think that for reasonable quantities of RAM this should be fine.

Of course I might be wrong :)

A simple test would be to write a dumb OS that reads/writes all mem but I couldn't be bothered :P

EDIT: I should add that I'm developing on Linux so I didn't actually compile and tried stuff with applepie unfortunately.

gamozolabs commented 5 years ago

Booting up in a Linux live CD and running memtest86 is a great test for this. You'll find that sadly this is not enough of a fix and memtest86+ will fail or even triple fault.

There are a few places in Bochs that explicitly mask 64-bit addresses with 0xffffffff and also cast addresses to Bit32us. I know it's possible to get this to work once enough of the places have been fixed, but until a very rigorous testing procedure is in place I cannot comfortably accept a pull request.

I'm just too concerned with unknowns here. With a good testing strategy put forward I'd be comfortable adding in patches if it seems the tests are fairly exhaustive of edge cases where this may fail.

thebabush commented 5 years ago

Thanks for taking the time.

My setup is bochs with BX_LARGE_RAMFILE=0 (aka no swapping of memory blocks) with 3gb of RAM. I'm 30% (Pass) in of Memtest86 v4.3.7 and so far everything is ok. This is basically a couple of physical machine hours that translate into 1 hour emulated time. I'm documenting here just to be sure, as I have to power down my machine unfortunately.

Did you see the fails/faults earlier than this or later?

I'm interested in bochs supporting > 2GB RAM so I might work on this... But I'm having troubles thinking about effective ways to test this stuff. An obvious way would be to write some long non-repeating sequence on all the address space and read it back, but that doesn't cover edge cases.

If you have any idea that you would like to share, I'm all ears. Feel free to close the PR, I'm just keeping it open because I dunno if you get notifications otherwise.

P.S.: I don't know why I said blocks are 4MB long... They clearly are 128K long.

gamozolabs commented 5 years ago

There is some code in misc_mem.cc that looks like:

#if BX_PHY_ADDRESS_LONG
    else if (a20addr > BX_CONST64(0xffffffff)) {
      // Error, requested addr is out of bounds.
      return (Bit8u *) &BX_MEM_THIS bogus[a20addr & 0xfff];
    }
#endif
#if BX_PHY_ADDRESS_LONG
    else if (addr > BX_CONST64(0xffffffff)) {
      *buf = 0xff;
      ret = 0; // error, beyond limits of memory
    }
#endif

In a few places. Things like this are what raise my concerns with whether this will behave corrcetly.

Did the memtest pass?

gamozolabs commented 5 years ago

I think the presence of a few of these leads me to uncertainty of where there might be others. That's really it.

thebabush commented 5 years ago

I didn't have time to run the memcheck completely, but it did work up until 30% and that means several complete passes were made. When I'll go back to a normal schedule I can leave it work over night to see if it ends completely, but I didn't see any error.

As for those two ifs, they are basically useless (it seems) because:

    else if(a20addr < BX_MEM_THIS len && ! is_bios)
    {
...
    }
#if BX_PHY_ADDRESS_LONG
    else if (a20addr > BX_CONST64(0xffffffff)) {
      // Error, requested addr is out of bounds.
      return (Bit8u *) &BX_MEM_THIS bogus[a20addr & 0xfff];
    }
#endif

and is_bios is false because if (a20addr > BX_CONST64(0xffffffff)) is_bios = 0;. It looks like the author of that code wanted to minimize changes / ifdefs and so patched in 64-bit support afterwards.

So a20addr > BX_CONST64(0xffffffff) is actually addr > memory_len.

Now, would I trust the rest to be correct? Dunno. I have been booting entire OSes (3gb RAM) without macro-level problems.

I understand and agree with your point. I'm not trying to get you to accept this PR :) I'm interested in hearing if you have non-manual-code-review related options to find potential problems out.

I guess asking on bochs' mailing list wouldn't hurt.

gamozolabs commented 5 years ago

Ah that's fair.

So if you're familiar with the E820 tables (this is how the BIOS tells the guest OS where memory is), it could be possible to maybe configure the BIOS to report very little memory available <4 GiB, and almost everything over 4 GiB. Give maybe like 32 MiB or something <4 GiB so the OS can get initial boot done but then an overwhelming majority of memory would be >4 GiB of RAM. This should force more of it to be used to increase the chances of a problem showing up if there is one.

This is the best thing I can think of for now. It's likely guests use physical memory in order from 0 to fff... so I would guess memory over 4 GiB is least likely to be hit.

Also if you tested with 3 GiB of RAM that probably won't stress 32-bit overflows. I'd probably want to use at least 8 GiB of RAM such that over half the memory will stress these potential issues.

I'm also concerned if BIOS things are appended to RAM and those might require that they're in a 32-bit address space. It might be necessary to create a break in memory to allow room for BIOS/option ROMs.

-B

thebabush commented 5 years ago

Nope, not familiar with them, but it might be time for me to learn something new :)

It looks like bochs' bios stuff is kind of hardcoded tbh.

The 3GB point is a good one and I'll run memcheck on 8gb for sure.

Btw, memory handlers seem kind of broken by big addresses. Dunno if a guest can map stuff in such a way that they cause problems.

Anyway thanks for taking the time, I'm closing the PR. Hopefully I'll have the time to work on this soon, as it seems to requires a decent amount of time.