diff mbox series

[OpenWrt-Devel,procd,2/4] system: fix failing image validation due to EINTR

Message ID 20200103004638.16307-3-ynezz@true.cz
State Accepted
Delegated to: Petr Štetiar
Headers show
Series fixes and improvements | expand

Commit Message

Petr Štetiar Jan. 3, 2020, 12:46 a.m. UTC
It was quite common to see following error during sysupgrade on serial
console:

 Failed to parse JSON: 4

This is happening due to the fact, that validate_firmware_image_call
fork()s then waits in blocking read() for the input from the child
process, but child finishes its tasks and exits, thus emitting SIGCHLD
signal which then leads to the interruption of the blocking read() in
the parent process with EINTR error.

It seems like the recent fixes in the libubox library, particulary in
the jshn sub-component (which empowers json_dump used in the shell
script executed by the child process) made the execution somehow faster,
thus exposing this racy behaviour in the validate_firmware_image_call at
least on RPi-4 (Cortex-A72) target.

So this patch fixes this issue by checking the read() return value and
retrying the read() if interrupted due to the EINTR error.

Ref: http://lists.infradead.org/pipermail/openwrt-devel/2020-January/020994.html
Fixes: e990e215e8a3 ("system: add "validate_firmware_image" ubus method")
Cc: Rafał Miłecki <rafal@milecki.pl>
Reported-by: Petr Novák <petrn@me.com>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 system.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Hauke Mehrtens Jan. 4, 2020, 5:46 p.m. UTC | #1
On 1/3/20 1:46 AM, Petr Štetiar wrote:
> It was quite common to see following error during sysupgrade on serial
> console:
> 
>  Failed to parse JSON: 4
> 
> This is happening due to the fact, that validate_firmware_image_call
> fork()s then waits in blocking read() for the input from the child
> process, but child finishes its tasks and exits, thus emitting SIGCHLD
> signal which then leads to the interruption of the blocking read() in
> the parent process with EINTR error.
> 
> It seems like the recent fixes in the libubox library, particulary in
> the jshn sub-component (which empowers json_dump used in the shell
> script executed by the child process) made the execution somehow faster,
> thus exposing this racy behaviour in the validate_firmware_image_call at
> least on RPi-4 (Cortex-A72) target.
> 
> So this patch fixes this issue by checking the read() return value and
> retrying the read() if interrupted due to the EINTR error.
> 
> Ref: http://lists.infradead.org/pipermail/openwrt-devel/2020-January/020994.html
> Fixes: e990e215e8a3 ("system: add "validate_firmware_image" ubus method")
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Reported-by: Petr Novák <petrn@me.com>
> Signed-off-by: Petr Štetiar <ynezz@true.cz>
> ---

Reviewed-by: Hauke Mehrtens <hauke@hauke-m.de>

>  system.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/system.c b/system.c
> index 65d3f09b7fb6..5cd88e0d8227 100644
> --- a/system.c
> +++ b/system.c
> @@ -466,6 +466,9 @@ static int validate_firmware_image_call(const char *file)
>  
>  	blob_buf_init(&b, 0);
>  	while ((len = read(fds[0], buf, sizeof(buf)))) {
> +		if (len < 0 && errno == EINTR)
> +			continue;
> +
>  		jsobj = json_tokener_parse_ex(tok, buf, len);
>  
>  		if (json_tokener_get_error(tok) == json_tokener_success)
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
diff mbox series

Patch

diff --git a/system.c b/system.c
index 65d3f09b7fb6..5cd88e0d8227 100644
--- a/system.c
+++ b/system.c
@@ -466,6 +466,9 @@  static int validate_firmware_image_call(const char *file)
 
 	blob_buf_init(&b, 0);
 	while ((len = read(fds[0], buf, sizeof(buf)))) {
+		if (len < 0 && errno == EINTR)
+			continue;
+
 		jsobj = json_tokener_parse_ex(tok, buf, len);
 
 		if (json_tokener_get_error(tok) == json_tokener_success)