Re: [PATCH] pasta: Save errno on signal handler entry, restore on return when needed

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

On Fri, Aug 02, 2024 at 03:34:48PM +0200, Stefano Brivio wrote:
> Ed reported this:
> 
>   # Error: pasta failed with exit code 1:
>   # Couldn't drop cap 3 from bounding set
>   # : No child processes
> 
> in a Podman CI run with tests being run in parallel. The error message
> itself, by the way, is fixed by commit 1cd773081f12 ("log: Drop
> newlines in the middle of the perror()-like messages"), but how can we
> possibly get ECHILD as failure code for prctl()?
> 
> Well, we don't, but if we exit early enough, pasta_child_handler()
> might run before we're even done with isolation steps, and it calls
> waitid(), which sets errno. We need to restore it before returning
> from the signal handler (if we return after calling functions that
> might set it), as signal-safety(7) also implies:
> 
>        Fetching and setting the value of errno is async-signal-safe
>        provided that the signal handler saves errno on entry and
>        restores its value before returning.
> 
> Eventually, we'll probably need to switch to signalfd(2) the day we
> want to implement multithreading, but this will do for the moment.
> 
> Reported-by: Ed Santiago <santiago@redhat.com>
> Link: https://github.com/containers/podman/issues/23478
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Ouch.  errno, another of those awful APIs we're stuck with :/

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  pasta.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/pasta.c b/pasta.c
> index 572048d..b4a3d99 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -61,6 +61,7 @@ int pasta_child_pid;
>   */
>  void pasta_child_handler(int signal)
>  {
> +	int errno_save = errno;
>  	siginfo_t infop;
>  
>  	(void)signal;
> @@ -85,6 +86,8 @@ void pasta_child_handler(int signal)
>  
>  	waitid(P_ALL, 0, NULL, WEXITED | WNOHANG);
>  	waitid(P_ALL, 0, NULL, WEXITED | WNOHANG);
> +
> +	errno = errno_save;
>  }
>  
>  /**

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]