[PATCH 0/1] MR8396: loader: Assert order in argv array
While the order that arguments are loaded by the kernel will never change, some emulators may push argument data in a different order which would cause a very large memset. The sys-v ABI doesn't define a specific order for the information itself, it only does so for the pointers. This assertion ensures the order of the arguments is the same as we expect from the Linux kernel. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396
From: Paris Oplopoios <parisoplop(a)gmail.com> While the order that arguments are loaded by the kernel will never change, some emulators may push argument pointers in a different order which would cause a very large memset. --- loader/preloader.c | 1 + 1 file changed, 1 insertion(+) diff --git a/loader/preloader.c b/loader/preloader.c index d0551bae63a..2ee36824303 100644 --- a/loader/preloader.c +++ b/loader/preloader.c @@ -1381,6 +1381,7 @@ static void set_process_name( int argc, char *argv[] ) /* find the end of the argv array and move everything down */ end = argv[argc - 1]; while (*end) end++; + if (argv[1] < argv[0]) fatal_error( "argv[1] is located before argv[0]\n" ); off = argv[1] - argv[0]; for (p = argv[1]; p <= end; p++) *(p - off) = *p; wld_memset( end - off, 0, off ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8396
AFAICS these CI failures are unrelated? What's generally done in this case, do I keep retrying? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396#note_107547
First of all, welcome to WineHQ and thanks for your contribution! By "doesn't define a specific order" I assume you're referring to this paragraph:
Argument strings, environment strings, and the auxiliary information appear in no spe- cific order within the information block and they need not be compactly allocated.[^1]
However, the *de facto* Linux ABI guarantees that the argument strings are, in fact, in the argv[] order. Why? On Linux, the information block page is shared with the kernel. When you modify argv strings, it is reflected in `/proc/<PID>/cmdline` so that tools like `top` and `htop` can inspect them. So, the emulator is broken. Note that this patch doesn't work on the emulator either: it crashes *with or without* the assertion. The emulator needs to be fixed if it wants to run *Linux* programs, not just SysV ABI-compliant ones. Hopefully it accepts contributions? [^1]: H.J. Lu et al., *System V Application Binary Interface AMD64 Architecture Processor Supplement (With LP64 and ILP32 Programming Models), Version 1.0.* §3.4.1 "Initial Stack and Register State," March 12, 2025. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396#note_107550
As a workaround, you can turn off preloader with `WINELOADERNOEXEC=1`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396#note_107551
On Sun Jun 22 16:00:20 2025 +0000, Jinoh Kang wrote:
First of all, welcome to WineHQ and thanks for your contribution! By "doesn't define a specific order" I assume you're referring to this paragraph:
Argument strings, environment strings, and the auxiliary information appear in no spe- cific order within the information block and they need not be compactly allocated.[^1] However, the *de facto* Linux ABI guarantees that the argument strings are, in fact, in the argv[] order. Why? On Linux, the information block page is shared with the kernel. When you modify argv strings, it is reflected in `/proc/<PID>/cmdline` so that tools like `top` and `htop` can inspect them. `cmdline` is a simple concatenation of argv strings delimited with NULL characters, and it always starts with argv[0]. The initial stack follows the same layout. The emulator is *not* respecting this layout guarantee. So, the emulator is broken. The emulator needs to be fixed if it wants to run *Linux* programs, not just SysV ABI-compliant ones. Hopefully it accepts contributions? Note that this patch doesn't work on the (broken) emulator either: it crashes *with or without* the assertion. [^1]: H.J. Lu et al., *System V Application Binary Interface AMD64 Architecture Processor Supplement (With LP64 and ILP32 Programming Models), Version 1.0.* §3.4.1 "Initial Stack and Register State," March 12, 2025. You are correct, in that a proper emulator should push these arguments to the information block in the same order as the Linux kernel. It is also the case that most programs don't actually rely on the order that the things in the information blocks are pushed.
The intention of this patch isn't to change this reliance. That's completely fine. It is an assert for future emulator developers that might land on this issue to save them a bit of debugging time. Yes, the emulator would be broken if it doesn't push them in the correct order. It has been fixed in my case. Personally it took me an hour or two to figure out what I was doing wrong, so I thought why not commit an assert in wine for anyone that might run into the same problem in the future? PS. If you think this is not something the wine project should care about, then I am fine with closing this merge request. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396#note_107552
On Sun Jun 22 16:12:01 2025 +0000, Paris Oplopoios wrote:
You are correct, in that a proper emulator should push these arguments to the information block in the same order as the Linux kernel. It is also the case that most programs don't actually rely on the order that the things in the information blocks are pushed. So in reality most Linux programs would work despite the order they push the argument strings in the information block. The intention of this patch isn't to change this reliance. That's completely fine. It is an assert for future emulator developers that might land on this issue to save them a bit of debugging time. Yes, the emulator would be broken if it doesn't push them in the correct order. It has been fixed in my case. Personally it took me an hour or two to figure out what I was doing wrong, so I thought why not commit an assert in wine for anyone that might run into the same problem in the future? PS. If you think this is not something the wine project should care about, then I am fine with closing this merge request. Wine *does* have many downstream wrappers and emulators (like Box64 and FEX-Emu), so it would be nice to make their jobs easier if possible.
That said, there are a million ways things can go wrong, and adding defensive asserts would make the code unreadable. For example, what about the argvs? PIE behaviors? prctl()s? Entry register states and/or TLS? Wine assumes that the platform works as expected, and it's emulator's job to ensure that. IMHO there are two kinds of bugs in the systems world: 1. *Deterministic bugs*: with proper tooling, segfaults like this would be easy to hunt down. To save time debugging, simply diff with the reference implementation (e.g., diff the stack layouts from emulator and stock Linux). 2. *Heisenbugs*: there are more difficult to diagnose. They are also much "rarer," so adding a bit of assertions for them wouldn't be much of an hassle (notwithstanding the point above). Preloader is single-threaded and pretty much deterministic. I'd be happy to see a separate project that tests/validates Linux ABI behaviors, so that other emulators like QEMU could also adopt them. Maybe it could already exist somewhere. But it's out of scope for Wine. So yeah, closing seems appropriate. Thanks for your contribution nonetheless! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396#note_107553
This merge request was closed by Paris Oplopoios. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396
On Sun Jun 22 16:12:01 2025 +0000, Jinoh Kang wrote:
Wine *does* have many downstream wrappers and emulators (like Box64 and FEX-Emu), so it would be nice to make their jobs easier if possible. That said, there are a million ways things can go wrong, and adding defensive asserts would make the code unreadable. For example, what about the argvs? PIE behaviors? prctl()s? Entry register states and/or TLS? Wine assumes that the platform works as expected, and it's emulator's job to ensure that. IMHO there are two kinds of bugs in the systems world: 1. *Deterministic bugs*: with proper tooling, segfaults like this would be easy to hunt down. To save time debugging, simply diff with the reference implementation (e.g., diff the stack layouts from emulator and stock Linux). 2. *Heisenbugs*: there are more difficult to diagnose. They are also much "rarer," so adding a bit of assertions for them wouldn't be much of an hassle (notwithstanding the point above). Preloader is single-threaded and pretty much deterministic. I'd be happy to see a separate project that tests/validates Linux ABI behaviors, so that other emulators like QEMU could also adopt them. Maybe it could already exist somewhere. But it's out of scope for Wine. So yeah, closing seems appropriate. Thanks for your contribution nonetheless! FWIW, if Wine didn't have to patch proc/*/cmdline, then I would have simply said "just shift the argv array itself." That also makes the code simpler (as opposed to an assertion). After all, modifying argv is not a common occurrence in most programs as you said.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8396#note_107555
participants (3)
-
Jinoh Kang (＠iamahuman) -
Paris Oplopoios
-
Paris Oplopoios (＠offtkp)