Saving and restoring errno in signal handlers

[hanna-kruppe]

This is probably a very unlikely issue to hit in practice, but since the library is otherwise very careful to do things by the book and document caveats, I wanted to raise it for consideration.

Signal handlers should save the current value of errno on entry and restore this value on exit if they call any functions that set errno. Otherwise, they will clobber errno for the thread being interrupted, which can cause problems if that thread was interrupted after calling a function that set errno but before checking the errno value. glibc documentation says:

Many functions that are AS-Safe may set errno, or modify the floating-point environment, because their doing so does not make them unsuitable for use in signal handlers. However, programs could misbehave should asynchronous signal handlers modify this thread-local state, and the signal handling machinery cannot be counted on to preserve it. Therefore, signal handlers that call functions that may set errno or modify the floating-point environment must save their original values, and restore them before returning.

POSIX-2024 (haven't checked earlier versions) also explicitly ties this to async-signal-safety:

Operations which obtain the value of errno and operations which assign a value to errno shall be async-signal-safe, provided that the signal-catching function saves the value of errno upon entry and restores it before it returns.

I believe this is not an issue for the bare signal handler installed by signal-hook-registry. It seems to only call errno-setting functions in the "NULL siginfo" error case, which aborts the process. However, the registered actions that are invoked in the signal handler need to worry about this. For example, the self-pipe trick as implemented in signal-hook calls write. This write usually shouldn't encounter errors, but making the write end of the pipe non-blocking means that errno will be clobbered if more signals are delivered while the pipe is already full (not common but certainly possible). It's also potentially relevant for anyone using signal-hook-registry directly -- I've become aware of this issue while once again scrutinizing my own signal handler using sem_post and sem_wait to implement thread suspension for a sampling profiler.

I think it would be great if signal-hook-registry could take care of the errno save-restore dance so that individual actions don't need to do it themselves. It's a fairly subtle issue that affects even code that carefully limits itself to async-signal-safe libc functions and it's easy to miss this requirement even when researching async-signal-safety. Unfortunately, implementing errno safe-restore in signal-hook-registry is not trivial:

• std::io::Error::last_os_error only allows reading errno, not restoring it.
• The libc crate only exposes the raw platform-specific guts underlying the errno macro, so it's hard to use portably.
• The errno crate deals with the platform specific differences to allow reading and writing, but its MSRV policy is different.
• Vendoring code from std or the errno crate that papers over differences in libc internals means one more copy of this code will have to be updated for future platforms.

If this is too much trouble, it would be great to at least fix the affected handlers defined in this repository and extend the documentation of signal_hook_registry::register to draw attention to this subtlety.

[vorner]

Hello

Actually, this is a really good catch. And possibly a bit more serious than you describe - I've seen a system that reset errno to 0 on a successful call. So we probably should find some solution.

As for the way how to do it. It should be possible to have a very small C file with just the two functions (or one, if we can read errno) implemented in C in the repository and use the cc crate. I do something similar for extracting signal details in the signal-hook crate. I guess the newest cc version needs newer rustc than this crate, but if anyone needs such old rustc with the newest version of this crate, they could probably find some older cc version that still works for them and pin that one (might need some experimentation to verify that). But I wonder if there are platforms that don't have a C library / runtime and if we support them in any way 🤔

Btw, what's the floating point environment? 😇

[hanna-kruppe]

As a user of signal-hook-registry, I'd prefer to avoid an unconditional dependency on compiling C code for this, if possible. I don't think it's a portability problem per se, but build scripts have a bunch of drawbacks and cc in particular has had and will likely continue to have its fair share of bugs and annoyances (understandable given the complexity of the problem it solves).

I would propose using errno = ">= 0.2, < 0.4". I had a closer look at its version history and it turns out that the 0.2 series has low enough MSRV for signal-hook-registry and the API is compatible enough with errno 0.3 that either version should work.

[hanna-kruppe]

Btw, what's the floating point environment? 😇

A big headache 🙃 Much harder to solve than errno, but also much smaller practical impact.

Rust doesn't allow changing the floating-point environment (fpenv), in fact it's UB to execute Rust code in a non-default fpenv. This means we don't have to worry about the registered actions changing the fpenv. However, it also means that if the code being interrupted by signal delivery has changed the fpenv or is inspecting status flags, then:

• Doing anything in the signal handler that raises FP exceptions may behave incorrectly, e.g., trap instead of setting status flags and giving default results.
• Even if FP exceptions only set status flags, this may cause the interrupted code to misbehave because it needs to inspect status flags which were clobbered by the signal handler.
• Executing any Rust code at all in the signal handler is technically UB. Even if no float operations are apparent in the source code, it's technically legal for the compiler to insert them anyway. Of course, trying to save and restore the fpenv anyway does nothing to solve the UB.

I believe the only fully correct way to solve this is write the entry point of the signal handler in assembly code or in C code using the proper tools for it (#pragma STD FENV_ACCESS and #include <fenv.h>). This entry point could save and reset all fpenv related state before entering Rust code, and reset it before returning. In practice, however:

• It's rare to use floating point arithmetic in signal handlers (unlike libc functions). It's also rare for LLVM to compile code in ways that introduces floating-point operations in code that doesn't seem to use any.
• The vast majority of C projects out there don't do this properly, either. I've never seen one that even bothers with saving/restoring fpenv state (but I did see at least one that saves/restores errno, and ThreadSanitizer has a check for this). Even if one tries to do it, they probably aren't using #pragma STD FENV_ACCESS or their compiler's equivalent, so it's still de jure UB.
• Doing it properly in C is not very portable, either, because Annex F is a C99 feature and support for it is still spotty and incomplete today. For example, Clang still doesn't claim full conformance (despite significant work in this direction in recent years) and intentionally doesn't conform on x86 targets without SSE2. So writing some standard-conforming C code to save, reset, and restore fpenv state probably won't compile or work properly on some supported targets.
• Assembly is the de jure correct path, but obviously highly platform-specific. It varies not just by ISA, but compiling assembly code as part of a Rust build is a much bigger pain than compiling C code.

Trying to save/restore in the signal handler written in Rust, while technically UB, may be a decent mitigation in practice. However, the C functions that would be useful for this aren't exposed by the libc crate, and probably not even available everywhere.

I don't think there's a good solution for this. It should at least be documented. But I don't see a portable way to mitigate it, let alone solve it. At best, one could try to do something for specific ISA and ABI combinations. Higher MSRV would help with this because it would give access to inline assembly and (as of Rust 1.88) naked functions.

[hanna-kruppe]

Another mitigation of the fpenv issue is that at least Linux on x86-64 already switches out MXCSR when invoking signal handlers. This doesn't seem clearly documented, but I found some old discussions on a mailing list archive and tested it experimentally. This makes some sense in that it's really hard to save/restore this properly in C, it's really easy to forget, and really hard to avoid doing anything FP-status-related (especially when memcpy may use SSE2). For what it's worth, the psABI says:

The control bits of the MXCSR register are callee-saved (preserved across calls), while the status bits are caller-saved (not preserved).

I'm not sure if this fully explains the observed behavior. At least a strict reading of the ABI would suggest anyone who modifies the control bits needs to restore them, which would include signal handlers. The warning in the glibc documentation might be referring to this aspects more than to status flags.

I'm even less sure if and how this extends to other operating systems and to Linux on other architectures. Nobody's really writing these things down clearly, other than the glibc warning quoted above. Arguably, POSIX can be read to imply that the OS/libc has to save/clear/restore fpenv when invoking signal handlers:

• POSIX includes (by reference) C's Annex F but does not seem to include any requirement w.r.t. fpenv and signal handlers, while the errno problem is mentioned explicitly.
• Various functions that are declared async-signal-safe are likely to be written in relatively ordinary C, not using #pragma STD FENV_ACCESS or compiler-specific flags with similar effects.
• The fe* functions that one would have to use to save/reset/restore fpenv manually are not listed as async-signal-safe, so there doesn't seem to be a POSIX-compliant alternative to "let the OS/libc take care of it".

It would be reasonable for the OS/libc to handle all of this, but I also wouldn't be surprised if there's some cases where an OS forgets (or deliberately decides against) saving/restoring some obscure bit of FPU-related state, e.g., because it's not expected to be changed and saving/restoring it would require extra work.

Further complication: Annex F and POSIX only give portable interfaces for FP exceptions, not for other modes such as denormals-are-zero, flush-to-zero, "fp exceptions cause traps", or "touching SIMD registers at all triggers a trap" modes of operation. You can save and restore the fpenv as a whole, which may include such processor-specific flags (it does on x86), but at that point you're well outside of standard C / POSIX.

Conclusion: welp, I'm more confused than before writing this comment, but at least the whole issue seems even more unlikely to be triggered.

[hanna-kruppe]

Some prior discussion of signal handlers and fpenv here: rust-lang/unsafe-code-guidelines#444

In particular, as pointed out there, C11 Annex J says:

The following are unspecified:

• [...] the state of the floating-point environment, when the processing of the abstract machine is interrupted by receipt of a signal (5.1.2.3).

[vorner]

Huh. I'd say that the fpenv is a very interesting chapter in its own name, probably best left for some time later. But it either seems to be „Very unlikely to be hit in practice“ or „Completely broken, because the compiler assumes it can do whatever it likes with this environment at any time whatsoever“. The latter one probably not reasonably fixable.

As for the your suggestion of using errno instead with the range of the versions, that would seem like a solution. Do you want to give it a try? I'm a bit short on free time :-|

As for MSRV, the documentation of signal-hook-registry is a bit vague in that regard … so if there was a good reason to increase the version, I guess it would be fair to do so to fix a bug. I'd prefer to keep the MSRV somewhat conservative even then, though.

[hanna-kruppe]

I'll make a quick PR for the errno thing, but I can't promise a lot of time for testing or unforeseen complications.