<div dir="ltr">Hi Germano,<div><br></div><div>no reason at all ... it just missed my attention, twice. Sorry.</div><div><br></div><div>This patch is now merged.</div><div>Thanks,</div><div>Christophe Varoqui</div><div><a href="http://www.opensvc.com">www.opensvc.com</a></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 18, 2016 at 12:42 PM, Germano Percossi <span dir="ltr"><<a href="mailto:germano.percossi@citrix.com" target="_blank">germano.percossi@citrix.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Christophe,<br>
<br>
Is there a specific reason this silly patch is not getting<br>
through?<br>
<br>
It seems a no-brainer to me but probably I am missing something.<br>
<br>
Thanks,<br>
Germano<span class="im HOEnZb"><br>
<br>
On 02/11/2016 07:30 PM, Germano Percossi wrote:<br>
</span><div class="HOEnZb"><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Within the reconfigure function, the global pointer conf is<br>
stored in a local variable and then assigned NULL.<br>
If load_config should fail, for any reason, we end up with<br>
a memory leak, as soon as we leave the function, and with<br>
the global pointer conf set to NULL, leading to a segfault<br>
as soon as it is dereferenced.<br>
<br>
I tested it by calling a reconfigure and making the first<br>
allocation in load_config fail but any failure in load_config<br>
would do.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>From a user perspective the CLI reports "fail".<br>
</blockquote>
<br>
If something like this should happen there are at least 2 possible<br>
scenarios:<br>
<br>
1) If a second immediate reconfigure succeeds, the conf now is fine but<br>
    the leak stays<br>
2) If the previous point does not happen, any command trying to access<br>
    "conf" would fail. On my test box a "show conf" segfaulted.<br>
<br>
The fix is simple but in case of failure at least the previous<br>
conf is kept in memory without leaks or segfaluts<br>
<br>
Signed-off-by: Germano Percossi <<a href="mailto:germano.percossi@citrix.com" target="_blank">germano.percossi@citrix.com</a>><br>
---<br>
  multipathd/main.c | 2 ++<br>
  1 file changed, 2 insertions(+)<br>
<br>
diff --git a/multipathd/main.c b/multipathd/main.c<br>
index 04f6d02..f83c849 100644<br>
--- a/multipathd/main.c<br>
+++ b/multipathd/main.c<br>
@@ -1551,6 +1551,8 @@ reconfigure (struct vectors * vecs)<br>
                configure(vecs, 1);<br>
                free_config(old);<br>
                retval = 0;<br>
+       } else {<br>
+               conf = old;<br>
        }<br>
<br>
        running_state = DAEMON_RUNNING;<br>
<br>
</blockquote>
</div></div></blockquote></div><br></div>