wtf icon indicating copy to clipboard operation
wtf copied to clipboard

Port I/O assertion in bochscpu causing client to crash

Open wumb0 opened this issue 1 year ago • 3 comments

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.

wumb0 avatar Sep 13 '24 16:09 wumb0

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

0vercl0k avatar Sep 13 '24 18:09 0vercl0k

Apologies for forgetting about this.

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

https://github.com/yrp604/bochscpu/commit/7dd2df8763fc441ab58b676becb0b82cdc682648 and https://github.com/yrp604/bochscpu-build/commit/9a6dcccbb85ac92ce3e031ace510a647fe2e1131

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?

yrp604 avatar Sep 24 '24 08:09 yrp604

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

wumb0 avatar Sep 30 '24 19:09 wumb0

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

Cheers

0vercl0k avatar Nov 03 '24 17:11 0vercl0k

Gentle ping! 🙂

Cheers

0vercl0k avatar Nov 26 '24 03:11 0vercl0k

All right I'm going to close this as this is stale -- feel free to re-open if you end up trying the patch / or if anybody needs this.

Cheers

0vercl0k avatar Feb 21 '25 15:02 0vercl0k