diff mbox series

[2/3] run_system_cmd: do not consider command over if child stopped

Message ID 20211119090401.664993-2-dominique.martinet@atmark-techno.com
State Accepted
Headers show
Series [1/3] fix missing field initializer warnings | expand

Commit Message

Dominique MARTINET Nov. 19, 2021, 9:04 a.m. UTC
waitpid() with WUNTRACED or WCONTINUED will return the pid if process has been
stopped or ptraced, which can lead to swupdate considering the command being run
before it actually is if something interacts with it.
We don't actually check WIFSTOPPED/WIFCONTINUED below and don't really care
about this state: just remove the flags to waitpid()

While we are here, only check WTERMSIG() if WIFSIGNALED is true, and add
a final neither-stopped-nor-killed state which probably should not see much
use

Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
Note: sigchld_handler does check WIFSTOPPED/WIFCONTINUED without
setting WUNTRACED/WCONTINUED flags to waitpid, so these should
never match in theory and could be removed

 core/pctl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Stefano Babic Nov. 21, 2021, 10:43 a.m. UTC | #1
Hi Dominique,

On 19.11.21 10:04, Dominique Martinet wrote:
> waitpid() with WUNTRACED or WCONTINUED will return the pid if process has been
> stopped or ptraced, which can lead to swupdate considering the command being run
> before it actually is if something interacts with it.
> We don't actually check WIFSTOPPED/WIFCONTINUED below and don't really care
> about this state: just remove the flags to waitpid()
> 
> While we are here, only check WTERMSIG() if WIFSIGNALED is true, and add
> a final neither-stopped-nor-killed state which probably should not see much
> use
> 
> Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
> ---
> Note: sigchld_handler does check WIFSTOPPED/WIFCONTINUED without
> setting WUNTRACED/WCONTINUED flags to waitpid, so these should
> never match in theory and could be removed
> 
>   core/pctl.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/core/pctl.c b/core/pctl.c
> index c31575aa309b..2a09d9807e3a 100644
> --- a/core/pctl.c
> +++ b/core/pctl.c
> @@ -328,7 +328,7 @@ int run_system_cmd(const char *cmd)
>   			fd_set readfds;
>   			int n, i;
>   
> -			w = waitpid(process_id, &wstatus, WNOHANG | WUNTRACED | WCONTINUED);
> +			w = waitpid(process_id, &wstatus, WNOHANG);
>   			if (w == -1) {
>   				ERROR("Error from waitpid() !!");
>   				close(stdoutpipe[PIPE_READ]);
> @@ -386,9 +386,12 @@ int run_system_cmd(const char *cmd)
>   		if (WIFEXITED(wstatus)) {
>   			ret = WEXITSTATUS(wstatus);
>   			TRACE("%s command returned %d", cmd, ret);
> -		} else {
> +		} else if (WIFSIGNALED(wstatus)) {
>   			TRACE("(%s) killed by signal %d\n", cmd, WTERMSIG(wstatus));
>   			ret = -1;
> +		} else {
> +			TRACE("(%s) not exited nor killed!\n", cmd);
> +			ret = -1;
>   		}
>   	}
>   
> 

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/core/pctl.c b/core/pctl.c
index c31575aa309b..2a09d9807e3a 100644
--- a/core/pctl.c
+++ b/core/pctl.c
@@ -328,7 +328,7 @@  int run_system_cmd(const char *cmd)
 			fd_set readfds;
 			int n, i;
 
-			w = waitpid(process_id, &wstatus, WNOHANG | WUNTRACED | WCONTINUED);
+			w = waitpid(process_id, &wstatus, WNOHANG);
 			if (w == -1) {
 				ERROR("Error from waitpid() !!");
 				close(stdoutpipe[PIPE_READ]);
@@ -386,9 +386,12 @@  int run_system_cmd(const char *cmd)
 		if (WIFEXITED(wstatus)) {
 			ret = WEXITSTATUS(wstatus);
 			TRACE("%s command returned %d", cmd, ret);
-		} else {
+		} else if (WIFSIGNALED(wstatus)) {
 			TRACE("(%s) killed by signal %d\n", cmd, WTERMSIG(wstatus));
 			ret = -1;
+		} else {
+			TRACE("(%s) not exited nor killed!\n", cmd);
+			ret = -1;
 		}
 	}