7 ms·

Murder Mystery: GCC builds failing after sbuild refactoring

141 points by pabs3 2y ago

rurban 2y ago
I had a similar murder mystery investigation the last months, and to rewrite the whole murder concept. But in the end, getting rid of signals (I hate signals ) and replacing it with messages didn't work, so I embraced arranging a proper group and killing them at once. With some exceptions.
IshKebab 2y ago
The standard Linux process management API is a total mess IMO. It doesn't even offer a way to avoid TOCTOU. They've made some improvements recently (https://lwn.net/Articles/794707/ https://lwn.net/Articles/794707/) but as far as I know you still can't implement something like pstree without just reading files from `/proc` which is shit.
- pjmlp 2y ago
  UNIX/POSIX process management.
  - deleted 2y ago
    [deleted]
- formerly_proven 2y ago
  Who needs handles for processes when you can have reusable IDs instead? /s For a while the limit has been around 4 million or something like that, instead of the traditional 32k. Which of course broke a bunch of stuff that assumed "char pid[5];" is good enough. Fun stuff. The kernel offers up this (here abbreviated) example which you could use for all accesses to /proc/<pid> when you have an open pidfd to ensure that the two refer to the same process: static int pidfd_metadata_fd(pid_t pid, int pidfd) { char path[100]; snprintf(path, sizeof(path), "/proc/%d", pid); int procfd = open(path, O_DIRECTORY | O_RDONLY | O_CLOEXEC); if (sys_pidfd_send_signal(pidfd, 0, NULL, 0) < 0 && errno != EPERM) { close(procfd); procfd = -1; } return procfd; } And these days you even can poll for pidfds (readable = exited) and you can even use P_PIDFD with waitid. So the very basic process management cycle can be done through pidfds nowadays. > as far as I know you still can't implement something like pstree without just reading files from `/proc` which is shit. It's also slow! Tools like top/htop/ps -aux etc. use a surprising amount of CPU because they have to do a trillion trips to the kernel and back. Compare this to the win32 equivalent where, iirc, there is just one or two trips to the kernel to acquire a snapshot and you walk that in user-space instead.
  - alexvitkov 2y ago
    char path[100]; snprintf(path, sizeof(path), "/proc/%d", pid); You're assuming the pid will fit in 93 bytes! The exact same mistake as `char pid[5];` Jokes aside, sometimes userspace deserves to be broken, and `char pid[5]` is IMO one of those cases. The fact that we treat integers as a scarce reusable resource is bad, and the fact that we now have a second-order process identifier to cover that up is worse.
    - formerly_proven 2y ago
      Fun fact that broken code was in the runtime used by a major Fortran compiler
- josephcsible 2y ago
  > It doesn't even offer a way to avoid TOCTOU. What do you mean? I admit there are cases where it's hard to avoid TOCTOU, but I can't think of any where it's impossible.
  - IshKebab 2y ago
    For example to kill all of the children of a process you first have to read all of the files in `/proc` to build a process tree, and then send your signals, by which time the process tree may have changed.
    - maccam94 2y ago
      Yeah, as far as I know the only way to be sure is by putting the parent process into a cgroup. Then to kill all of the child processes you have to freeze the cgroup, enumerate the pids, send sigterm/sigkill to all of them, before unfreezing again.
    - josephcsible 2y ago
      Doesn't the new pidfd API you linked give you a race-free way to do that?
      - IshKebab 2y ago
        Maybe. Tbh I was trying to do something like that on RHEL8 and I think it's too old for procfd.
    - o11c 2y ago
      As long as you aren't trying to reimplement `systemd` badly, you really shouldn't have to deal with this.
      - IshKebab 2y ago
        "As long as you aren't trying to use the flawed API you won't run into any of its flaws." Ok then.
- ijustlovemath 2y ago
  With TOCTOU, the solution is often just to try to use the resource and handle errors, not to assume that since you checked it, it's available.
  - loeg 2y ago
    PID reuse is the classic example here. There is no neat solution other than process handles, which Unix/Linux does not historically have.
    - fc417fc802 2y ago
      Aren't process handles still an integer under the hood in the end? The solution is presumably the same - use enough bits.
      - int_19h 2y ago
        What makes process handles different is that they have a lifetime of their own that is distinct from the lifetime of the process - even if the process exits, the handle still refers to that now-dead process, not to some random new process, until you release the handle.
        fc417fc802 2y ago
        But if it's still an integer (that you can't see) then it's still subject to reuse to the exact same extent that an integer is or isn't. It's entirely possible to wrap your internal integer at 16 bits and experience handle collisions. Alternatively you are free to use full 64 bit integers on Linux (at least IIUC). If you can architect a system that never experiences handle collision then quite trivially you can architect an equivalent system that never experiences PID reuse.
        int_19h 2y ago
        You're missing the point. No, handles are not subject to such a problem because their lifetimes are explicit and distinct from the underlying resource. E.g. in Win32, if you have a process handle, it will not ever refer to a process different from the one that it was originally created for until you specifically call CloseHandle on it. The OS will retain that handle in its table and will not reuse it for any other process. And yes, of course it would be trivially easy to do the same in Linux. Except for that whole part where it would break all the existing code relying on POSIX semantics.
- Polizeiposaune 2y ago
  The problem here wasn't a case of TOCTOU; rather, the program was using a magic constant value that was outside the range of getpid() but which wasn't outside of the domain of kill(2)'s first parameter.
  - nneonneo 2y ago
    Proper enum types would have solved this problem; e.g. in Rust enum ProcessHandle { Pid(pid_t), Invalid, Terminated } There is no way to pass a Terminated or Invalid process handle to kill, since there is no numeric identifier for it anymore. And, any code that wants to get a PID from a ProcessHandle has to explicitly handle non-PIDs - making it impossible to forget to deal with these special cases.
    - orf 2y ago
      While correct, everything is a number at the ABI boundary, so it’s still possible - if not harder. Depending on the layout and arch, -2 could well represent Terminated
      - nneonneo 2y ago
        I think this misunderstands how Rust enums work. The discriminator is separate from the payload, unless niches are involved (they probably wouldn’t be here - note that -2 is a valid pid_t). You cannot go and pass a Rust enum to any FFI function directly (without extra annotations, at least); you’d have to specifically extract the pid_t from the enum before passing it to an FFI function like kill(). And you can only obtain a pid_t from the Pid variant; if the enum holds a different variant, the compiler will stop you from trying to pull out a pid_t.
        orf 2y ago
        I understand how Rust enums work, but you’re misunderstanding how ABIs work (or my original point, or both). Rust enums are a Rust concept. Two identical enums will have different memory layouts on different machines, Rust versions, etc. Even if this was solved, you’re still passing the enum across a boundary. Both sides need to agree on the layout of the enum in memory. To illustrate the problem, imagine we had two variants: `Pid(NonZeroUsize), LaunchTheNukes` This can be laid out in memory with a single `usize`: the 0 value becomes the discriminate for `LaunchTheNukes`. Thus, if you pass `0` (I.e null) across the boundary the other side will interpret it as `LaunchTheNukes`. In short, if you naively change the layout of an enum and don’t recompile everything that uses the enum (i.e the kernel) then your -2 value could change from being interpreted as `Pid(-2)` in the application to `LaunchTheNukes` in the kernel. It gets more complex with larger enums and more discriminators, but the general point is that the concepts of enums, type safety and such doesn’t really exist across boundaries like syscalls, because it doesn’t exist in hardware / at the machine level.
- ajross 2y ago
  Heh, "even". BSD process groups were around for two decades at least before anyone even thought of the idea of a TOCTOU vulnerability, much less coined an acronym for it. You're just asking for too much here. Yes, it's an API you probably shouldn't use. No, it's not going away. The solution for this particular problem is cgroups. The build system just wasn't using it.
  - wahern 2y ago
    What inherent TOCTOU vulnerability exists with process groups? Process groups are the way to shoot down a collection of processes atomically in Unix. getpgrp returns the current processes' process group, and killpg(pgrp) sends a signal atomically; so long as the process doesn't switch process groups between those two calls (and using normal APIs only the process can do this itself), there's no race. (IOW, there's no race in `killpg(getpgrp(), signo)`) There are other ways, too, like using SIGHUP delivered when a controlling TTY is closed (not strictly POSIX, but reliable semantics across all the major extant Unix-like systems). cgroups also allows killing a group of processes race-free, but Linux only got this feature (cgroup.kill) in 2021.[1] Before then even systemd would just walk the cgroup PID list and send a signal to each individual process, which was inherently racy. If this was done from PID 1--init--this could maybe be done correctly by ensuring zombie's weren't reaped--preventing PID recycling--while walking the PID list. OTOH, Linux supports a prctl, PR_SET_CHILD_SUBREAPER, to change a reaper, so perhaps even systemd could never guaranteed to not accidentally shoot down an innocent process. [1] Merged https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/kernel/cgroup/cgroup.c?id=661ee6280931548f7b3b887ad26a157474ae5ac4 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux... I'm not sure what the subsequent release history looks like, but I wouldn't be surprised if some popular LTS distro versions lack this capability.
    - ajross 2y ago
      I suspect the upstream concern was dealing with services that themselves like to spawn process groups. There's no hierarchy of process groups, once you call setpgid() you've escaped the sandbox and the API can't help your parents manage you anymore. The use case in question is a build system that by its nature needs to deal with foreign software. Also FWIW, you could freeze a cgroup atomically in the very earliest versions and deconstruct it at leisure. You just didn't do it by sending signals.
usr1106 2y ago
Excellent writeup! Some observations unrelated to the bug. 1. Nice to see some details about the Debian build server implementation. I have always found it rather obscure. Compared to that OpenSUSE's OBS is refreshingly open. Not only can you just use their cloud service for free, but you can also rather easily set up your own instance. 2. Nice to see that the Debian build infra gets updated. I am a bit suprised that Perl code gets introduced in 2024. Isn't that a bit out of fashion for a reason: It's typically very hard to read code written by others. (As said unrelated. It did not cause the bug in TFA.)
- Kwpolska 2y ago
  sbuild is written in Perl, so it’s not surprising that new code for it is written in Perl.
- aragilar 2y ago
  I believe installing https://packages.debian.org/sid/sbuild-debian-developer-setup https://packages.debian.org/sid/sbuild-debian-developer-setu... will get you most of the way (but I've only ever done manual builds of packages, so I may be wrong).
- deleted 2y ago
  [deleted]
- chubot 2y ago
  Are you surprised that programs can live for 10 or 20 years? Most people who want to rewrite things in a language that's more fashionable don't have the experience maintaining working systems over 20+ years, like Debian does
  - eyegor 2y ago
    I'll just pitch in as someone who's worked with several 40+ year old codebases and 3+ year old perl, the perl code is significantly harder to maintain. The language lends itself to unreadable levels of terseness and abuse of regex. Unless you use perl every day, the amount of time trying to understand the syntax vs the code logic is skewed too heavily towards obscure syntax. Even the oldest fortran/c is much easier to work with. Except maybe arithmetic gotos designed to minimize the number of functions. Those are straight evil and I'm glad no modern language supports them.
dwattttt 2y ago
It's problems like these (mistaking a sentinel value for a valid member of a type) that make me think sentinel values are just a mistake. In-band signalling must have been responsible for so many bugs at this point.
- ajross 2y ago
  It's hard though. I mean, it's easy to say you shouldn't do this. But that becomes isomorphic to "You shouldn't have NULL". And... that's hard, because you really need NULLs for a lot of problems. Or "You shouldn't have error code returns from syscalls", when you really need those. It's possible to split it out, but then everyone complains about the result. We thought exceptions were going to fix the error code problem, but then we collectively decided we hate them. OSes and static analyzers and instrumentation tools like valgrind/asan/msan have done a really good job of turning NULL dereference into a built-in assertion everywhere, but people won't use them. Even Rust, which has probably come closest to a genuine solution, has done so only at the cost of immense complexity and somewhat fugly syntax. No free lunch. I think what's notable here isn't the bug itself, which is pretty routine. It's the fact that simple bugs can be buried so deeply that they get exposed only in a six-hour-long test rig on one particular Linux distribution's backend.
  - dwattttt 2y ago
    Rust's answer is its type system, but Rust is hardly the only language that can do this. If a syscalls returns an fd, and on error it returns a negative value, why not define it as a bitfield with one bit a flag? Then in C there'd at least be a field name you could grep for, or grep for it not being there.
    - AlotOfReading 2y ago
      Let's steelman this and ignore the portability issues C has with bitfields on ABIs. Many processors don't have a BFI instruction, so working with a bitfield will generate multiple instructions. At the scale of a large program, this can easily result in measurable code bloat and significant (single digit percent) performance overheads on the happy path. This is actually a discussion that's been going on in the systems programming community for decades. There's a cultural aversion to the runtime costs that's always outweighed any hygiene advantages.
mrguyorama 2y ago
>From the kill(2) man page >If pid is less than -1, then sig is sent to every process in the process group whose ID is -pid. This is just awful IMO. Any time you build an API that has significant behavior boundary on input value (rather than input type) you have probably made a mistake. I assume this is from the 60s though and therefore impossible to change.
- int_19h 2y ago
  This is still very common, unfortunately. Look at all the languages adopting special semantics for negative indices, for example - Python's xs[-1] etc. This can be a lot of fun to debug when you're passing some computed N, and the bug is that it happens to be negative in some cases.
rleigh 2y ago
Good to see schroot is being replaced in sbuild, given that it's nearly 20 years old and no longer a cutting-edge solution to the problem of creating build environments. Can't believe time flies that fast since I wrote it in 2005! Has an outright replacement for sbuild itself been considered? Given the ubiquity of modern CI systems, it seems quite possible to replace it entirely. I would have thought GitLab runners or Jenkins or any other standard CI system would work well. While I once enjoyed writing Perl, sbuild is not a great piece of code, and the problems it was trying to solve now have better and more widely-used solutions which don't require ongoing maintenance of an even older piece of Perl code.

141 points by pabs3 2y ago

I had a similar murder mystery investigation the last months, and to rewrite the whole murder concept. But in the end, getting rid of signals (I hate signals ) and replacing it with messages didn't work, so I embraced arranging a proper group and killing them at once. With some exceptions.

IshKebab 2y ago

The standard Linux process management API is a total mess IMO. It doesn't even offer a way to avoid TOCTOU. They've made some improvements recently (https://lwn.net/Articles/794707/ https://lwn.net/Articles/794707/) but as far as I know you still can't implement something like pstree without just reading files from `/proc` which is shit.

pjmlp 2y ago
UNIX/POSIX process management.
- deleted 2y ago
  [deleted]
formerly_proven 2y ago
Who needs handles for processes when you can have reusable IDs instead? /s For a while the limit has been around 4 million or something like that, instead of the traditional 32k. Which of course broke a bunch of stuff that assumed "char pid[5];" is good enough. Fun stuff. The kernel offers up this (here abbreviated) example which you could use for all accesses to /proc/<pid> when you have an open pidfd to ensure that the two refer to the same process: static int pidfd_metadata_fd(pid_t pid, int pidfd) { char path[100]; snprintf(path, sizeof(path), "/proc/%d", pid); int procfd = open(path, O_DIRECTORY | O_RDONLY | O_CLOEXEC); if (sys_pidfd_send_signal(pidfd, 0, NULL, 0) < 0 && errno != EPERM) { close(procfd); procfd = -1; } return procfd; } And these days you even can poll for pidfds (readable = exited) and you can even use P_PIDFD with waitid. So the very basic process management cycle can be done through pidfds nowadays. > as far as I know you still can't implement something like pstree without just reading files from `/proc` which is shit. It's also slow! Tools like top/htop/ps -aux etc. use a surprising amount of CPU because they have to do a trillion trips to the kernel and back. Compare this to the win32 equivalent where, iirc, there is just one or two trips to the kernel to acquire a snapshot and you walk that in user-space instead.
- alexvitkov 2y ago
  char path[100]; snprintf(path, sizeof(path), "/proc/%d", pid); You're assuming the pid will fit in 93 bytes! The exact same mistake as `char pid[5];` Jokes aside, sometimes userspace deserves to be broken, and `char pid[5]` is IMO one of those cases. The fact that we treat integers as a scarce reusable resource is bad, and the fact that we now have a second-order process identifier to cover that up is worse.
  - formerly_proven 2y ago
    Fun fact that broken code was in the runtime used by a major Fortran compiler
josephcsible 2y ago
> It doesn't even offer a way to avoid TOCTOU. What do you mean? I admit there are cases where it's hard to avoid TOCTOU, but I can't think of any where it's impossible.
- IshKebab 2y ago
  For example to kill all of the children of a process you first have to read all of the files in `/proc` to build a process tree, and then send your signals, by which time the process tree may have changed.
  - maccam94 2y ago
    Yeah, as far as I know the only way to be sure is by putting the parent process into a cgroup. Then to kill all of the child processes you have to freeze the cgroup, enumerate the pids, send sigterm/sigkill to all of them, before unfreezing again.
  - josephcsible 2y ago
    Doesn't the new pidfd API you linked give you a race-free way to do that?
    - IshKebab 2y ago
      Maybe. Tbh I was trying to do something like that on RHEL8 and I think it's too old for procfd.
  - o11c 2y ago
    As long as you aren't trying to reimplement `systemd` badly, you really shouldn't have to deal with this.
    - IshKebab 2y ago
      "As long as you aren't trying to use the flawed API you won't run into any of its flaws." Ok then.
ijustlovemath 2y ago
With TOCTOU, the solution is often just to try to use the resource and handle errors, not to assume that since you checked it, it's available.
- loeg 2y ago
  PID reuse is the classic example here. There is no neat solution other than process handles, which Unix/Linux does not historically have.
  - fc417fc802 2y ago
    Aren't process handles still an integer under the hood in the end? The solution is presumably the same - use enough bits.
    - int_19h 2y ago
      What makes process handles different is that they have a lifetime of their own that is distinct from the lifetime of the process - even if the process exits, the handle still refers to that now-dead process, not to some random new process, until you release the handle.
      - fc417fc802 2y ago
        But if it's still an integer (that you can't see) then it's still subject to reuse to the exact same extent that an integer is or isn't. It's entirely possible to wrap your internal integer at 16 bits and experience handle collisions. Alternatively you are free to use full 64 bit integers on Linux (at least IIUC). If you can architect a system that never experiences handle collision then quite trivially you can architect an equivalent system that never experiences PID reuse.
        int_19h 2y ago
        You're missing the point. No, handles are not subject to such a problem because their lifetimes are explicit and distinct from the underlying resource. E.g. in Win32, if you have a process handle, it will not ever refer to a process different from the one that it was originally created for until you specifically call CloseHandle on it. The OS will retain that handle in its table and will not reuse it for any other process. And yes, of course it would be trivially easy to do the same in Linux. Except for that whole part where it would break all the existing code relying on POSIX semantics.
Polizeiposaune 2y ago
The problem here wasn't a case of TOCTOU; rather, the program was using a magic constant value that was outside the range of getpid() but which wasn't outside of the domain of kill(2)'s first parameter.
- nneonneo 2y ago
  Proper enum types would have solved this problem; e.g. in Rust enum ProcessHandle { Pid(pid_t), Invalid, Terminated } There is no way to pass a Terminated or Invalid process handle to kill, since there is no numeric identifier for it anymore. And, any code that wants to get a PID from a ProcessHandle has to explicitly handle non-PIDs - making it impossible to forget to deal with these special cases.
  - orf 2y ago
    While correct, everything is a number at the ABI boundary, so it’s still possible - if not harder. Depending on the layout and arch, -2 could well represent Terminated
    - nneonneo 2y ago
      I think this misunderstands how Rust enums work. The discriminator is separate from the payload, unless niches are involved (they probably wouldn’t be here - note that -2 is a valid pid_t). You cannot go and pass a Rust enum to any FFI function directly (without extra annotations, at least); you’d have to specifically extract the pid_t from the enum before passing it to an FFI function like kill(). And you can only obtain a pid_t from the Pid variant; if the enum holds a different variant, the compiler will stop you from trying to pull out a pid_t.
      - orf 2y ago
        I understand how Rust enums work, but you’re misunderstanding how ABIs work (or my original point, or both). Rust enums are a Rust concept. Two identical enums will have different memory layouts on different machines, Rust versions, etc. Even if this was solved, you’re still passing the enum across a boundary. Both sides need to agree on the layout of the enum in memory. To illustrate the problem, imagine we had two variants: `Pid(NonZeroUsize), LaunchTheNukes` This can be laid out in memory with a single `usize`: the 0 value becomes the discriminate for `LaunchTheNukes`. Thus, if you pass `0` (I.e null) across the boundary the other side will interpret it as `LaunchTheNukes`. In short, if you naively change the layout of an enum and don’t recompile everything that uses the enum (i.e the kernel) then your -2 value could change from being interpreted as `Pid(-2)` in the application to `LaunchTheNukes` in the kernel. It gets more complex with larger enums and more discriminators, but the general point is that the concepts of enums, type safety and such doesn’t really exist across boundaries like syscalls, because it doesn’t exist in hardware / at the machine level.
ajross 2y ago
Heh, "even". BSD process groups were around for two decades at least before anyone even thought of the idea of a TOCTOU vulnerability, much less coined an acronym for it. You're just asking for too much here. Yes, it's an API you probably shouldn't use. No, it's not going away. The solution for this particular problem is cgroups. The build system just wasn't using it.
- wahern 2y ago
  What inherent TOCTOU vulnerability exists with process groups? Process groups are the way to shoot down a collection of processes atomically in Unix. getpgrp returns the current processes' process group, and killpg(pgrp) sends a signal atomically; so long as the process doesn't switch process groups between those two calls (and using normal APIs only the process can do this itself), there's no race. (IOW, there's no race in `killpg(getpgrp(), signo)`) There are other ways, too, like using SIGHUP delivered when a controlling TTY is closed (not strictly POSIX, but reliable semantics across all the major extant Unix-like systems). cgroups also allows killing a group of processes race-free, but Linux only got this feature (cgroup.kill) in 2021.[1] Before then even systemd would just walk the cgroup PID list and send a signal to each individual process, which was inherently racy. If this was done from PID 1--init--this could maybe be done correctly by ensuring zombie's weren't reaped--preventing PID recycling--while walking the PID list. OTOH, Linux supports a prctl, PR_SET_CHILD_SUBREAPER, to change a reaper, so perhaps even systemd could never guaranteed to not accidentally shoot down an innocent process. [1] Merged https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/kernel/cgroup/cgroup.c?id=661ee6280931548f7b3b887ad26a157474ae5ac4 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux... I'm not sure what the subsequent release history looks like, but I wouldn't be surprised if some popular LTS distro versions lack this capability.
  - ajross 2y ago
    I suspect the upstream concern was dealing with services that themselves like to spawn process groups. There's no hierarchy of process groups, once you call setpgid() you've escaped the sandbox and the API can't help your parents manage you anymore. The use case in question is a build system that by its nature needs to deal with foreign software. Also FWIW, you could freeze a cgroup atomically in the very earliest versions and deconstruct it at leisure. You just didn't do it by sending signals.

usr1106 2y ago

Excellent writeup! Some observations unrelated to the bug. 1. Nice to see some details about the Debian build server implementation. I have always found it rather obscure. Compared to that OpenSUSE's OBS is refreshingly open. Not only can you just use their cloud service for free, but you can also rather easily set up your own instance. 2. Nice to see that the Debian build infra gets updated. I am a bit suprised that Perl code gets introduced in 2024. Isn't that a bit out of fashion for a reason: It's typically very hard to read code written by others. (As said unrelated. It did not cause the bug in TFA.)

Kwpolska 2y ago
sbuild is written in Perl, so it’s not surprising that new code for it is written in Perl.
aragilar 2y ago
I believe installing https://packages.debian.org/sid/sbuild-debian-developer-setup https://packages.debian.org/sid/sbuild-debian-developer-setu... will get you most of the way (but I've only ever done manual builds of packages, so I may be wrong).
deleted 2y ago
[deleted]
chubot 2y ago
Are you surprised that programs can live for 10 or 20 years? Most people who want to rewrite things in a language that's more fashionable don't have the experience maintaining working systems over 20+ years, like Debian does
- eyegor 2y ago
  I'll just pitch in as someone who's worked with several 40+ year old codebases and 3+ year old perl, the perl code is significantly harder to maintain. The language lends itself to unreadable levels of terseness and abuse of regex. Unless you use perl every day, the amount of time trying to understand the syntax vs the code logic is skewed too heavily towards obscure syntax. Even the oldest fortran/c is much easier to work with. Except maybe arithmetic gotos designed to minimize the number of functions. Those are straight evil and I'm glad no modern language supports them.

dwattttt 2y ago

It's problems like these (mistaking a sentinel value for a valid member of a type) that make me think sentinel values are just a mistake. In-band signalling must have been responsible for so many bugs at this point.

ajross 2y ago
It's hard though. I mean, it's easy to say you shouldn't do this. But that becomes isomorphic to "You shouldn't have NULL". And... that's hard, because you really need NULLs for a lot of problems. Or "You shouldn't have error code returns from syscalls", when you really need those. It's possible to split it out, but then everyone complains about the result. We thought exceptions were going to fix the error code problem, but then we collectively decided we hate them. OSes and static analyzers and instrumentation tools like valgrind/asan/msan have done a really good job of turning NULL dereference into a built-in assertion everywhere, but people won't use them. Even Rust, which has probably come closest to a genuine solution, has done so only at the cost of immense complexity and somewhat fugly syntax. No free lunch. I think what's notable here isn't the bug itself, which is pretty routine. It's the fact that simple bugs can be buried so deeply that they get exposed only in a six-hour-long test rig on one particular Linux distribution's backend.
- dwattttt 2y ago
  Rust's answer is its type system, but Rust is hardly the only language that can do this. If a syscalls returns an fd, and on error it returns a negative value, why not define it as a bitfield with one bit a flag? Then in C there'd at least be a field name you could grep for, or grep for it not being there.
  - AlotOfReading 2y ago
    Let's steelman this and ignore the portability issues C has with bitfields on ABIs. Many processors don't have a BFI instruction, so working with a bitfield will generate multiple instructions. At the scale of a large program, this can easily result in measurable code bloat and significant (single digit percent) performance overheads on the happy path. This is actually a discussion that's been going on in the systems programming community for decades. There's a cultural aversion to the runtime costs that's always outweighed any hygiene advantages.

mrguyorama 2y ago

>From the kill(2) man page >If pid is less than -1, then sig is sent to every process in the process group whose ID is -pid. This is just awful IMO. Any time you build an API that has significant behavior boundary on input value (rather than input type) you have probably made a mistake. I assume this is from the 60s though and therefore impossible to change.

int_19h 2y ago
This is still very common, unfortunately. Look at all the languages adopting special semantics for negative indices, for example - Python's xs[-1] etc. This can be a lot of fun to debug when you're passing some computed N, and the bug is that it happens to be negative in some cases.

rleigh 2y ago

Good to see schroot is being replaced in sbuild, given that it's nearly 20 years old and no longer a cutting-edge solution to the problem of creating build environments. Can't believe time flies that fast since I wrote it in 2005! Has an outright replacement for sbuild itself been considered? Given the ubiquity of modern CI systems, it seems quite possible to replace it entirely. I would have thought GitLab runners or Jenkins or any other standard CI system would work well. While I once enjoyed writing Perl, sbuild is not a great piece of code, and the problems it was trying to solve now have better and more widely-used solutions which don't require ongoing maintenance of an even older piece of Perl code.