Skip to content

Fix dynamic SCSI buffer window sizing causing SEGV#6128

Merged
jriwanek merged 1 commit into
86Box:masterfrom
awilfox:awilfox/scsi-fix
Sep 6, 2025
Merged

Fix dynamic SCSI buffer window sizing causing SEGV#6128
jriwanek merged 1 commit into
86Box:masterfrom
awilfox:awilfox/scsi-fix

Conversation

@awilfox

@awilfox awilfox commented Sep 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

I am one of the maintainers of the Adélie Linux distribution. One of our users was trying to run Adélie beta6 in 86Box and was finding that 86Box was crashing during disk formatting when using a SCSI controller.

This is because Linux has a dynamic window buffer for some SCSI controllers, and 86Box thinks the length should always be the same once a buffer has been allocated.

Affects 4.2.1, 5.0, and main, at least. Happy to discuss more at length if needed/desired.

Checklist

  • I have discussed this with core contributors already
  • This pull request requires changes to the ROM set Nope, just a simple buffer sizing fix.

References

Linux kernel source.

ASAN report 

=================================================================
==117171==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x515000e94d00 at pc 0x562d1f267e2d bp 0x7f002e8d8460 sp 0x7f002e8d7c30
WRITE of size 512 at 0x515000e94d00 thread T28
    #0 0x562d1f267e2c in __asan_memset (/home/user/code/86Box-4.2.1/b/src/86Box+0x502e2c)
    #1 0x562d2084334e in mvhd_sparse_read /home/user/code/86Box-4.2.1/src/disk/minivhd/minivhd_io.c:254:13
    #2 0x562d1ff0c25c in hdd_image_read /home/user/code/86Box-4.2.1/src/disk/hdd_image.c:510:35
    #3 0x562d1ffd4c47 in scsi_disk_blocks /home/user/code/86Box-4.2.1/src/scsi/scsi_disk.c:733:13
    #4 0x562d1ffce5e7 in scsi_disk_command /home/user/code/86Box-4.2.1/src/scsi/scsi_disk.c:1040:19
    #5 0x562d1ffa98d4 in scsi_device_target_command /home/user/code/86Box-4.2.1/src/scsi/scsi_device.c:39:9
    #6 0x562d1ffa98d4 in scsi_device_command_phase0 /home/user/code/86Box-4.2.1/src/scsi/scsi_device.c:119:19
    #7 0x562d1ffdc1d9 in x54x_scsi_cmd /home/user/code/86Box-4.2.1/src/scsi/scsi_x54x.c:936:5
    #8 0x562d1ffdc1d9 in x54x_cmd_callback /home/user/code/86Box-4.2.1/src/scsi/scsi_x54x.c:1259:13
    #9 0x562d1f2c8400 in timer_process /home/user/code/86Box-4.2.1/src/timer.c:137:13
    #10 0x562d1f977706 in exec386_dynarec /home/user/code/86Box-4.2.1/src/cpu/386_dynarec.c:851:21
    #11 0x562d1f2b281b in pc_run /home/user/code/86Box-4.2.1/src/86box.c:1426:5
    #12 0x562d20683fd9 in main_thread_fn() /home/user/code/86Box-4.2.1/src/qt/qt_main.cpp:122:13
    #13 0x7f009a4e64e3  (/usr/lib/gcc/x86_64-pc-linux-gnu/14/libstdc++.so.6+0xe64e3)
    #14 0x562d1f265bdf in asan_thread_start(void*) asan_interceptors.cpp.o
    #15 0x7f009a215a68  (/usr/lib64/libc.so.6+0x8aa68)
    #16 0x7f009a281eab  (/usr/lib64/libc.so.6+0xf6eab)

0x515000e94d00 is located 0 bytes after 512-byte region [0x515000e94b00,0x515000e94d00)
allocated by thread T28 here:
    #0 0x562d1f26a9a3 in malloc (/home/user/code/86Box-4.2.1/b/src/86Box+0x5059a3)
    #1 0x562d1ffcedbe in scsi_disk_buf_alloc /home/user/code/86Box-4.2.1/src/scsi/scsi_disk.c:627:40
    #2 0x562d1ffcedbe in scsi_disk_command /home/user/code/86Box-4.2.1/src/scsi/scsi_disk.c:1177:13
    #3 0x562d1ffa98d4 in scsi_device_target_command /home/user/code/86Box-4.2.1/src/scsi/scsi_device.c:39:9
    #4 0x562d1ffa98d4 in scsi_device_command_phase0 /home/user/code/86Box-4.2.1/src/scsi/scsi_device.c:119:19
    #5 0x562d1f2c8400 in timer_process /home/user/code/86Box-4.2.1/src/timer.c:137:13

Thread T28 created by T0 here:
    #0 0x562d1f24dab5 in pthread_create (/home/user/code/86Box-4.2.1/b/src/86Box+0x4e8ab5)
    #1 0x7f009a4e65b8 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State>>, void (*)()) (/usr/lib/gcc/x86_64-pc-linux-gnu/14/libstdc++.so.6+0xe65b8)
    #2 0x7f009aeb2057 in QObject::event(QEvent*) /var/tmp/portage/dev-qt/qtcore-5.15.14/work/qtbase-everywhere-src-5.15.14/src/corelib/kernel/qobject.cpp:1347:31
    #3 0x7f009b902c41 in QApplicationPrivate::notify_helper(QObject*, QEvent*) (/usr/lib64/libQt5Widgets.so.5+0x102c41)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/user/code/86Box-4.2.1/b/src/86Box+0x502e2c) in __asan_memset
Shadow bytes around the buggy address:
  0x515000e94a80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x515000e94b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x515000e94b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x515000e94c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x515000e94c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x515000e94d00:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x515000e94d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x515000e94e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x515000e94e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x515000e94f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x515000e94f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==117171==ABORTING

@jriwanek
jriwanek merged commit 41dcee2 into 86Box:master Sep 6, 2025
47 checks passed
@OBattler

OBattler commented Sep 6, 2025

Copy link
Copy Markdown
Member

Going to merge this and then apply the same changes also to CD-ROM, removable disk, and MO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhrdlicka dhrdlicka dhrdlicka approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@awilfox @OBattler @dhrdlicka @jriwanek