This is the mail archive of the glibc-bugs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug libc/22273] Improper assert in Linux posix_spawn implementation


https://sourceware.org/bugzilla/show_bug.cgi?id=22273

--- Comment #3 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Florian Weimer from comment #0)
> __spawnix has this code:
> 
>     340   /* Child must set args.err to something non-negative - we rely on
>     341      the parent and child sharing VM.  */
>     342   args.err = -1;
> …
>     354   /* The clone flags used will create a new child that will run in
> the same
>     355      memory space (CLONE_VM) and the execution of calling thread
> will be
>     356      suspend until the child calls execve or _exit.
>     357 
>     358      Also since the calling thread execution will be suspend, there
> is not
>     359      need for CLONE_SETTLS.  Although parent and child share the
> same TLS
>     360      namespace, there will be no concurrent access for TLS variables
> (errno
>     361      for instance).  */
>     362   new_pid = CLONE (__spawni_child, STACK (stack, stack_size),
> stack_size,
>     363                    CLONE_VM | CLONE_VFORK | SIGCHLD, &args);
>     364 
>     365   if (new_pid > 0)
>     366     {
>     367       ec = args.err;
>     368       assert (ec >= 0);
> 
> The assert can fire if the child process dies before the err member is
> written.  Atomics should probably used to guard against compiler reordering
> of the store, too.

(In reply to Florian Weimer from comment #2)
> (In reply to Adhemerval Zanella from comment #1)
> > So the input err memory position won't be accessed concurrently on parent
> > because CLONE_VFORK (since kernel interface guarantee that calling process
> > is suspended) and all return path on the child functions will be either
> > default execution through args->exec/maybe_script_execute or through the
> > fail label.
> 
> But if the process is killed, this will not result in a vfork failure, will
> it?

Indeed killing the child in the middle of the its execution might trigger the
assert.  Another issue I can see is process might be killed after line 271
(setting err to 0) and before actually issuing exec.  In this case it will
posix_spawn won't trigger the assert and returns as the external process has
ran correctly.

However I do not think setting the err atomically would help here, ideally we
will need kernel help on to signaling abrupt child execution. One option would
be to set all signals to SIG_IGN (although it won't help with SIGKILL or
SIGSTOP).

What about removing the assert and setting all signal handler as SIG_IGN?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]