Our website uses cookies to enhance your browsing experience.
In September, we broke down the 86Box v5.0 release, timed for the 30th anniversary of Windows 95 retail launch, and promised to show you something else. What is it that we promised? And why did we save our discovery for a separate article?
86Box is a "hardware-accurate" emulator for IBM PCs and compatibles. In our previous article, we looked through potential bugs, partly relying on the technical documentation of the emulated components. We also mentioned that the project was built in the RelWithDebInfo configuration—a release build with debugging information included. This information now becomes useful as our virtual tantalum capacitor exploded while we were digging through PVS-Studio warnings.
First, let's welcome the main character of today's episode—the PVS-Studio analyzer warning:
V575 The null pointer is passed into 'fseek' function. Inspect the first argument. vid_ati_eeprom.c 61
void
ati_eeprom_load_mach8(ati_eeprom_t *eeprom, char *fn, int mca)
{
FILE *fp;
....
fp = nvr_fopen(eeprom->fn, "rb");
size = 128;
if (!fp) {
if (mca) {
(void) fseek(fp, 2L, SEEK_SET); // <=
memset(eeprom->data + 2, 0xff, size - 2);
fp = nvr_fopen(eeprom->fn, "wb");
fwrite(eeprom->data, 1, size, fp);
....
}We need to load data stored in the video adapter's NVRAM, which we keep in a binary file. If the file doesn't exist, we need to create it with "default" data. Let's look at the case where the file is missing. We shift the file pointer, but it's null. As a result, we get an fp null pointer dereference.
Let's take a closer look at fseek. The C11 standard doesn't define requirements for the function first parameter and doesn't guarantee a check for NULL. This means it's up to the standard library developers to handle it properly. Now let's welcome:
The last two C standard library implementations are here as guests: 86Box is not designed to work with them, or their compatibility hasn't been checked. The build instructions don't mention alternative implementations either. So, let's start with the expected standard libraries and ask them to repeat the same actions with a null file pointer.
We take an IBM PS/2 model 55SX from the imaginary shelf and "plug in" the IBM 8514/A 2D accelerator made by ATI.
The first test subject is a Windows instance built using MinGW. We ensure the NVRAM file is absent before starting—we check the %userprofile%\86Box VMs\<virtual machine name>\nvr directory for the ati8514_mca.nvr file. If it is there, we delete it.
Turning on the power supply, and...
Nothing exploded! Everything is fine: the NVRAM file is written, the computer is running, and the smoke test on glibc is complete. No defect detected.
Moving on to FreeBSD. The libc library implements the standard C library in this OS. This is generally true for all BSD-family operating systems.
We use the same configuration. We check for the absence of the ati8514_mca.nvr NVRAM file at the ~/.local/share/86Box/Virtual Machines/<virtual machine name>/nvr path. Three, two, one, power is on...
Well, only an event from Ben Grubbs' recent past could better describe this situation :)
After opening our eyes, squeezed shut after the explosion, we look at the console: we have confirmed an abnormal exit!
void VMManagerSystem::launchMainProcess() Full Command:
"/root/86Box/build_freebsd/src/86Box"
("--vmpath", "/root/.local/share/86Box/Virtual Machines/somevm",
"--vmname",
"somevm")
Connection received on 86Box.socket.5876c5
Connection disconnected
Abnormal program termination while launching main process:
exit code 11, exit status QProcess::CrashExitA core dump file appeared next to the emulator executable. Let's welcome LLDB:
root@freebsd:~/86Box/build_freebsd/src # lldb 86Box -c 86Box.core
(lldb) target create "86Box" --core "86Box.core"
Core file '/root/86Box/build_freebsd/src/86Box.core' (x86_64) was loaded.
(lldb) bt
* thread #1, name = '86Box', stop reason = signal SIGSEGV
* frame #0: 0x0000000832f880bf
libc.so.7`_flockfile(fp=0x0000000000000000)
at _flock_stub.c:65:20
frame #1: 0x0000000832f8b675
libc.so.7`fseek(fp=0x0000000000000000, offset=2, whence=0)
at fseek.c:62:2
frame #2: 0x00000000018cd964
86Box`ati_eeprom_load_mach8(eeprom=...., fn=<unavailable>, mca=1)
at vid_ati_eeprom.c:61:20The fp null pointer makes a spectacular fire show—there was no way to lock the file as its descriptor is invalid. Unfortunately, LLDB didn't really want to work in real time, crashing either with a quiet "lost connection" or with a loud boom and special effects. Therefore, I can't show you how the code is executed like in Windows.
How did it happen that the function works correctly in one library but doesn't work in another? Let's now look at the fseek structure in glibc and BSD libc. We'll have to navigate through some macros.
glibc:
1. fp is passed to the CHECK_FILE macro in fseek.c.
int
fseek (FILE *fp, long int offset, int whence)
{
int result;
CHECK_FILE (fp, -1); // <=
_IO_acquire_lock (fp);
result = _IO_fseek (fp, offset, whence);
_IO_release_lock (fp);
return result;
}2. Inside the CHECK_FILE macro from libioP.h with FILE... nothing happens?
#ifdef IO_DEBUG
# define CHECK_FILE(FILE, RET) do { \
if ((FILE) == NULL \
|| ((FILE)->_flags & _IO_MAGIC_MASK) != _IO_MAGIC) \
{ \
__set_errno (EINVAL); \
return RET; \
} \
} while (0)
#else
# define CHECK_FILE(FILE, RET) do { } while (0)
#endifWell, not exactly. At the very least, it sets the return value to -1 in the MinGW version of glibc. But here's what happens in glibc from Devuan 6 "Excalibur"...
Purely by chance, I found a relevant discussion about a similar issue on the Debian bugs-devel mailing list. We conclude that the function behavior also depends on how glibc was compiled. What an unpleasant surprise!
What's even more alarming is how the function return value was handled: it was silenced by casting to void.
(void) fseek(fp, 2L, SEEK_SET);glibc tolerated this treatment on MinGW, but on GNU/Linux, it showed its strength and struck without warning. And no one can prove it wrong because, let me remind you: the standard doesn't define how the fseek function behaves with a null file pointer!
BSD libc:
1. fp is passed to the FLOCKFILE_CANCELSAFE macro in fseek.c.
int
fseek(FILE *fp, long offset, int whence)
{
int ret;
int serrno = errno;
/* make sure stdio is set up */
if (!__sdidinit)
__sinit();
FLOCKFILE_CANCELSAFE(fp); // <=
ret = _fseeko(fp, (off_t)offset, whence, 1);
FUNLOCKFILE_CANCELSAFE();
if (ret == 0)
errno = serrno;
return (ret);
}2. fp is passed to the _FLOCKFILE macro in local.h.
#define FLOCKFILE_CANCELSAFE(fp) \
{ \
struct _pthread_cleanup_info __cleanup_info__; \
if (__isthreaded) { \
_FLOCKFILE(fp); \ // <=
___pthread_cleanup_push_imp( \
__stdio_cancel_cleanup, (fp), \
&__cleanup_info__); \
} else { \
___pthread_cleanup_push_imp( \
__stdio_cancel_cleanup, NULL, \
&__cleanup_info__); \
} \
{
#define FUNLOCKFILE_CANCELSAFE() \
(void)0; \
} \
___pthread_cleanup_pop_imp(1); \
}3. The macro expands into the call to the _flockfile function in _flock_stub.c.
#ifdef _FLOCK_DEBUG
#define _FLOCKFILE(x) _flockfile_debug(x, __FILE__, __LINE__)
#else
#define _FLOCKFILE(x) _flockfile(x)
#endifAnd on the third step, we also have a null pointer dereference.
void
_flockfile(FILE *fp)
{
pthread_t curthread = _pthread_self();
if (fp->_fl_owner == curthread) // <=
fp->_fl_count++;
else {
....
}
}What shall we do now? Believe it or not, we should not touch the null pointer—fp is reused later in the code to open the file for writing anyway. We just need to delete the line.
void
ati_eeprom_load_mach8(ati_eeprom_t *eeprom, char *fn, int mca)
{
FILE *fp;
....
fp = nvr_fopen(eeprom->fn, "rb");
size = 128;
if (!fp) {
if (mca) {
memset(eeprom->data + 2, 0xff, size - 2);
fp = nvr_fopen(eeprom->fn, "wb");
fwrite(eeprom->data, 1, size, fp);
....
}Let's put the imaginary soldering iron back in its stand, rebuild the "test bench" once more, and three, two, one, power is on...
We have a video signal!
No more crashes. We have reached the expected state for the PS/2 model 55SX, requiring BIOS setup.
Note that the adjacent ati_eeprom_load_mach8_vga function does exactly that, immediately reopening the file for writing.
The soldering iron is put away, the soldering flux is cleaned. Now we can talk about the remaining two C standard library analogues.
Let's continue our show by demonstrating the behavior of Microsoft universal version—the Microsoft Universal C Runtime. Take a look inside the Windows SDK 10.0.26100:
static int __cdecl common_fseek(
__crt_stdio_stream const stream,
__int64 const offset,
int const whence,
__crt_cached_ptd_host& ptd
) throw()
{
_UCRT_VALIDATE_RETURN(ptd, stream.valid(), EINVAL, -1);
....
}
extern "C" int __cdecl fseek(
FILE* const public_stream,
long const offset,
int const whence
)
{
__crt_cached_ptd_host ptd;
return common_fseek(__crt_stdio_stream(public_stream), offset, whence, ptd);
}The _UCRT_VALIDATE_RETURN macro rejects any attempt to pass an invalid file descriptor to the function, and in the Release configuration, the application crashes with an exception:
Unhandled exception at 0x00007FFAB796CBA8 (ucrtbase.dll) in example.exe: An invalid parameter was passed to a function that considers invalid parameters fatal.
Thus, we already have three possible outcomes for the code without a check: glibc gives us a "compilation-dependent" scenario, BSD libc has a full-blown meltdown over unchecked data, and UCRT responds with righteous indignation.
What about musl?
1. The fseek function transitions to the __fseeko function in fseek.c, then to the FLOCK macro to block the file.
int __fseeko(FILE *f, off_t off, int whence)
{
int result;
FLOCK(f); // <=
result = __fseeko_unlocked(f, off, whence);
FUNLOCK(f);
return result;
}
int fseek(FILE *f, long off, int whence)
{
return __fseeko(f, off, whence);
}2. The f argument in the FLOCK macro in stdio_impl.h, which is a pointer to the file descriptor, is dereferenced without checking.
#define FLOCK(f) int __need_unlock = ((f)->lock>=0 ? __lockfile((f)) : 0)The BSD libc scenario repeats itself! Is this some kind of conspiracy? Not at all. Let me reiterate: if something is not defined by the standard, relying on the implementation is harmful to the program health. There is no other way.
We've found and resolved the issue—the graphics adapter works again in the problematic environment. The capability to detect such issues using static analysis is a strong argument in its favor, including for regular use scenarios. The FreeBSD build went through several fixes after the 86Box v5.0 release. So, the v5.1 version now provides a working program right out of the box.
This concludes the "repair." Thank you for your time, and see you in the next article!
0
0
0
2802
0
56
0
8489
1
0