util: virExec may blocked by reading pipe if grandchild prematurely exit

[PATCH] util: virExec may blocked by reading pipe if grandchild prematurely exit

Posted by Yi Wang 2 years, 5 months ago

From: Xu Chao <xu.chao6@zte.com.cn>

When VIR_EXEC_DAEMON is set, if virPidFileAcquirePath/virSetInherit failed,
then pipesync[0] can not be closed when granchild process exit, because
pipesync[1] still opened in child process. and then saferead in child
process may blocked forever, and left grandchild process in defunct state.

Signed-off-by: Xu Chao <xu.chao6@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
 src/util/vircommand.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index fead373729..fa71f40d81 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -782,6 +782,9 @@ virExec(virCommand *cmd)
         }
 
         if (pid > 0) {
+            /* close pipe[1], then the pipe can be closed if grandchild
+             * died prematurely */
+            VIR_FORCE_CLOSE(pipesync[1]);
             /* The parent expect us to have written the pid file before
              * exiting. Wait here for the child to write it and signal us. */
             if (cmd->pidfile &&
-- 
2.27.0

Re: [PATCH] util: virExec may blocked by reading pipe if grandchild prematurely exit

Posted by Michal Prívozník 2 years, 5 months ago

On 11/24/21 03:33, Yi Wang wrote:
> From: Xu Chao <xu.chao6@zte.com.cn>
> 
> When VIR_EXEC_DAEMON is set, if virPidFileAcquirePath/virSetInherit failed,
> then pipesync[0] can not be closed when granchild process exit, because
> pipesync[1] still opened in child process. and then saferead in child
> process may blocked forever, and left grandchild process in defunct state.
> 
> Signed-off-by: Xu Chao <xu.chao6@zte.com.cn>
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> ---
>  src/util/vircommand.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index fead373729..fa71f40d81 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -782,6 +782,9 @@ virExec(virCommand *cmd)
>          }
>  
>          if (pid > 0) {
> +            /* close pipe[1], then the pipe can be closed if grandchild
> +             * died prematurely */
> +            VIR_FORCE_CLOSE(pipesync[1]);
>              /* The parent expect us to have written the pid file before
>               * exiting. Wait here for the child to write it and signal us. */
>              if (cmd->pidfile &&
> 

I've tweaked the comment a bit to describe who has the write end open.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed. Congratulations on your first libvirt contribution!

Michal