Re: [PATCH 1/2] BUG/MINOR: mworker: Prevent potential use-after-free in mworker_env_to_proc_list

[Tim Düsterhus]

William,

Am 14.05.19 um 11:40 schrieb William Lallemand:
> Sorry, I'm only reading this mail and I already fixed this one in the master!
> 

All good, at least it's fixed. Take a look at the other patch (the
memory leak), though!

Best regards
Tim Düsterhus

Re: [PATCH 1/2] BUG/MINOR: mworker: Prevent potential use-after-free in mworker_env_to_proc_list

[William Lallemand]

Hi Tim,

On Mon, May 13, 2019 at 02:37:24PM +0200, Tim Duesterhus wrote:
> This was found by reading the code while investigating issue #96 and not
> verified with any tools:
> 
> If `child->pid` is falsy `child` will be freed instead of being added to
> `proc_list`. The setting of `PROC_O_LEAVING` happens unconditionally after
> this check.
> 
> Fix the issue by mising the setting of the LEAVING option right behind the
> allocation of `child`.
> 
> This bug was introduced in 4528611ed66d8bfa344782f6c7f1e7151cf48bf5, which
> is specific to the 2.0-dev branch. No backport required.
 
Sorry, I'm only reading this mail and I already fixed this one in the master!

-- 
William Lallemand

[PATCH 1/2] BUG/MINOR: mworker: Prevent potential use-after-free in mworker_env_to_proc_list

[Tim Duesterhus]

This was found by reading the code while investigating issue #96 and not
verified with any tools:

If `child->pid` is falsy `child` will be freed instead of being added to
`proc_list`. The setting of `PROC_O_LEAVING` happens unconditionally after
this check.

Fix the issue by mising the setting of the LEAVING option right behind the
allocation of `child`.

This bug was introduced in 4528611ed66d8bfa344782f6c7f1e7151cf48bf5, which
is specific to the 2.0-dev branch. No backport required.
---
 src/mworker.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mworker.c b/src/mworker.c
index 8df748d3f..491d40837 100644
--- a/src/mworker.c
+++ b/src/mworker.c
@@ -147,6 +147,9 @@ void mworker_env_to_proc_list()
 
child = calloc(1, sizeof(*child));
 
+   /* this is a process inherited from a reload that should be 
leaving */
+   child->options |= PROC_O_LEAVING;
+
while ((subtoken = strtok_r(token, ";", ))) {
 
token = NULL;
@@ -184,10 +187,7 @@ void mworker_env_to_proc_list()
} else {
free(child->id);
free(child);
-
}
-   /* this is a process inherited from a reload that should be 
leaving */
-   child->options |= PROC_O_LEAVING;
}
 
unsetenv("HAPROXY_PROCESSES");
-- 
2.21.0