diff mbox series

[OpenWrt-Devel] procd sysupgrade: close input side of pipe before reading

Message ID EFB73C49-4DE9-4DF7-93FF-72318E9C4526@temperednetworks.com
State Changes Requested
Delegated to: Rafał Miłecki
Headers show
Series [OpenWrt-Devel] procd sysupgrade: close input side of pipe before reading | expand

Commit Message

Dustin Lundquist Oct. 3, 2019, 4:24 p.m. UTC
When /usr/libexec/validate_firmware_image is not present on the system
procd will hang indefinitely on the read() since the input side of the
pipe is still open.

Signed-off-by: Dustin Lundquist <d.lundquist@temperednetworks.com>
---
system.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rafał Miłecki Oct. 27, 2019, 1:44 p.m. UTC | #1
On Thu, 3 Oct 2019 at 18:24, Dustin Lundquist
<D.Lundquist@temperednetworks.com> wrote:
> When /usr/libexec/validate_firmware_image is not present on the system
> procd will hang indefinitely on the read() since the input side of the
> pipe is still open.
>
> Signed-off-by: Dustin Lundquist <d.lundquist@temperednetworks.com>
> ---
> system.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/system.c b/system.c
> index 751a016..a7ada13 100644
> --- a/system.c
> +++ b/system.c
> @@ -425,6 +425,7 @@ static int validate_firmware_image_call(const char *file)
>         }
>
>         /* Parent process */
> +       close(fds[1]);
>
>         tok = json_tokener_new();
>         if (!tok) {
> @@ -447,7 +448,6 @@ static int validate_firmware_image_call(const char *file)
>         }
>
>         close(fds[0]);
> -       close(fds[1]);
>
>         err = -ENOENT;
>         if (jsobj) {

You also need to drop close(fds[1]); that is placed inside the "if
(!tok)" block.
Dustin Lundquist Oct. 28, 2019, 4:52 p.m. UTC | #2
> On Oct 27, 2019, at 6:44 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> 
> You also need to drop close(fds[1]); that is placed inside the "if
> (!tok)" block.


When /usr/libexec/validate_firmware_image is not present on the system
procd will hang indefinitely on the read() since the input side of the
pipe is still open.

Also fix pipe file descriptor leak when fork() fails.

Signed-off-by: Dustin Lundquist <d.lundquist@temperednetworks.com>
---
system.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/system.c b/system.c
index 751a016..98d9f8c 100644
--- a/system.c
+++ b/system.c
@@ -405,6 +405,8 @@ static int validate_firmware_image_call(const char *file)

	switch (fork()) {
	case -1:
+		close(fds[0]);
+		close(fds[1]);
		return -errno;
	case 0:
		/* Set stdin & stderr to /dev/null */
@@ -425,11 +427,11 @@ static int validate_firmware_image_call(const char *file)
	}

	/* Parent process */
+	close(fds[1]);

	tok = json_tokener_new();
	if (!tok) {
		close(fds[0]);
-		close(fds[1]);
		return -ENOMEM;
	}

@@ -447,7 +449,6 @@ static int validate_firmware_image_call(const char *file)
	}

	close(fds[0]);
-	close(fds[1]);

	err = -ENOENT;
	if (jsobj) {
--
2.20.1
Rafał Miłecki Oct. 28, 2019, 10:03 p.m. UTC | #3
On Mon, 28 Oct 2019 at 17:52, Dustin Lundquist
<D.Lundquist@temperednetworks.com> wrote:
> > On Oct 27, 2019, at 6:44 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> >
> > You also need to drop close(fds[1]); that is placed inside the "if
> > (!tok)" block.
>
>
> When /usr/libexec/validate_firmware_image is not present on the system
> procd will hang indefinitely on the read() since the input side of the
> pipe is still open.
>
> Also fix pipe file descriptor leak when fork() fails.
>
> Signed-off-by: Dustin Lundquist <d.lundquist@temperednetworks.com>

Thanks!

Acked-by: Rafał Miłecki <rafal@milecki.pl>
diff mbox series

Patch

diff --git a/system.c b/system.c
index 751a016..a7ada13 100644
--- a/system.c
+++ b/system.c
@@ -425,6 +425,7 @@  static int validate_firmware_image_call(const char *file)
	}

	/* Parent process */
+	close(fds[1]);

	tok = json_tokener_new();
	if (!tok) {
@@ -447,7 +448,6 @@  static int validate_firmware_image_call(const char *file)
	}

	close(fds[0]);
-	close(fds[1]);

	err = -ENOENT;
	if (jsobj) {