Port I/O assertion in bochscpu causing client to crash

[wumb0]

Hello! Just documenting this here because I ran into it while fuzzing a driver: port IO (in/out instructions) under bochs cause the fuzz client to crash due to a debug assertion in yrp604/bochscpu.

Assertion failed: false, file cabi/devices-cabi.cc, line 7

Which points to this file that just hardcodes an assert(false):
https://github.com/yrp604/bochscpu/blob/5182099c74816c06d7ad4240b5ccf1fe60ca975a/cabi/devices-cabi.cc#L7

I don't really expect you to do anything about it, since it's not in your code, but I wanted to put it here just so others know it's something they might run into. Port I/O emulation is probably outside the scope of this project, but it would be nice to have a way to at least gracefully recover if port I/O is encountered in the target code, instead of crashing the client completely.

I'll noodle on a fix, but might submit a PR over at bochscpu to allow port I/O hooks or something. Open to suggestions.

[0vercl0k]

Hello,

Thanks for doing that, really appreciate it 🙏🏽

Adding @yrp604 who might have ideas on 'would not crashing be a good idea' / what to do in those situations.

Cheers

[yrp604]

Apologies for forgetting about this.

I've pushed a small, untested sample patch here:

yrp604/bochscpu@7dd2df8
and
yrp604/bochscpu-build@9a6dccc

Basic idea is simple, instead of aborting we just call out to the existing inp2/outp hooks and let them handle the calls. Looking at https://github.com/bochs-emu/Bochs/blob/3ca671ca808cf0d586ebf2d4021d734e4a2142d0/bochs/iodev/devices.cc#L1052 I don't see a ton of other state we should need to update on the bochs side to keep things consistent here.

This should allow a plugin author to adjust the system state as desired during in/out calls, along with modifying the values that get returned. The ergonomics of this at the moment need improvement -- if a user doesn't specify this hook I'd like to find a nice way to abort if these callbacks are triggered.

Obviously we could do the same thing with the inp callback instead of only covering inp2 if needed.

Can you give this (or an approach like this) a try to see if it addresses your use case?

[wumb0]

Giving it a shot today. I'll post a PR for the hooks.

[0vercl0k]

Circling back on this - @wumb0 did you get a chance to try out @yrp604's patch?

Cheers