From: Denis V. Lunev <[email protected]> ide_drive_pio_post_load() validates end_transfer_fn_idx but takes cur_io_buffer_offset and cur_io_buffer_len straight from the migration stream, so data_ptr and data_end can be placed anywhere within +-2GB of the 131076-byte io_buffer allocation. Both fields are signed 32-bit.
The subsection loader consumes every subsection present in the stream without consulting needed(), so a crafted stream can inject ide_drive/pio_state for a drive that was never in a DRQ state. Once data_end is out of bounds, ide_data_writew() only compares the guest's pointer against that same bogus data_end, and the resumed guest turns a repeated outw to the data port into a controlled 16-bit heap write. end_transfer_fn_idx picks the direction, so the read side of the same code path leaks host heap instead. Validate the window against io_buffer_total_len and fail the load. The subtraction form avoids overflowing the addition. Reported-by: XlabAI Team of Tencent Xuanwu Lab <[email protected]> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4179 Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3738 Cc: John Snow <[email protected]> Cc: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Denis V. Lunev <[email protected]> --- hw/ide/core.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/ide/core.c b/hw/ide/core.c index 8190549ee8..0dca2b5c52 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2898,6 +2898,12 @@ static int ide_drive_pio_post_load(void *opaque, int version_id) if (s->end_transfer_fn_idx >= ARRAY_SIZE(transfer_end_table)) { return -EINVAL; } + if (s->cur_io_buffer_offset < 0 || s->cur_io_buffer_len < 0 || + s->cur_io_buffer_offset > s->io_buffer_total_len || + s->cur_io_buffer_len > + s->io_buffer_total_len - s->cur_io_buffer_offset) { + return -EINVAL; + } s->end_transfer_func = transfer_end_table[s->end_transfer_fn_idx]; s->data_ptr = s->io_buffer + s->cur_io_buffer_offset; s->data_end = s->data_ptr + s->cur_io_buffer_len; -- 2.53.0
