Summary

A guest that drives the PLIP parallel port adapter can make 86Box write past the end of the device allocation, which shows up as random corruption or a crash rather than as a PLIP error.

The transmit path takes the frame length from four guest written nibbles (src/network/net_plip.c:151 and :159), so tx_len can be anything up to 0xFFFF, and the data phase keeps storing bytes while dev->tx_ptr < dev->tx_len (src/network/net_plip.c:192). The guard that should stop the store is written as if (LIKELY(dev->tx_ptr) < sizeof(dev->tx_pkt)) at src/network/net_plip.c:173 and :182. The closing parenthesis sits after dev->tx_ptr, so the comparison operates on the result of __builtin_expect(!!(dev->tx_ptr), 1), which is 0 or 1 and always less than 1518. The check is therefore always true and the store happens for every announced byte. tx_pkt is the last member of plip_t (src/network/net_plip.c:79), so the extra bytes land outside the allocation, and the else branch that logs tx_pkt[%d] = overflow at src/network/net_plip.c:187 can never run.

This only bites on GCC and clang builds. Under the fallback #define LIKELY(x) (x) at src/include/86box/86box.h:98 the same text parses the way it was meant to.

The change moves the closing parenthesis on both lines so LIKELY wraps the whole comparison. Nothing else is needed. Valid frames are unaffected, the checksum still accumulates over the stored bytes only, and the send side was already safe because network_queue_put drops anything longer than NET_MAX_FRAME (src/network/network.c:339). The receive path already has the correct form of the same clamp at src/network/net_plip.c:410.

Checked two ways. First, a standalone harness that copies the two PLIP_TX_DATA cases and the real buffer sizes, with the device struct placed so its last byte ends against an unmapped guard page: with the guard as written it traps on the write to tx_pkt[1518]; with the parenthesis moved it consumes all 65535 announced bytes and writes nothing past tx_pkt[1517]. Second, clang -fsyntax-only -Wall -Wextra -Wsign-compare -std=gnu11 on net_plip.c, which reported comparison of integers of different signs: 'long' and 'unsigned long' at 173:37 and 182:37 before the change and neither afterwards, with no new warnings. I did not run 86Box itself for this.

The guard was added in a7a9ab4, in the same hunk that replaced the per packet calloc() with the fixed tx_pkt[NET_MAX_FRAME] array.

Checklist

• I have tested my changes locally and validated that the functionality works as intended

References

https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html for __builtin_expect, which returns the value of its first argument and so takes part in the surrounding expression.

a7a9ab4 for the commit that introduced the guard.