* [PATCH 1/1] multipath: fix memory leak and segfault in reconfigure
@ 2016-02-11 19:30 Germano Percossi
2016-02-23 15:17 ` Germano Percossi
2016-04-18 10:42 ` Germano Percossi
0 siblings, 2 replies; 4+ messages in thread
From: Germano Percossi @ 2016-02-11 19:30 UTC (permalink / raw)
To: christophe.varoqui; +Cc: dm-devel
Within the reconfigure function, the global pointer conf is
stored in a local variable and then assigned NULL.
If load_config should fail, for any reason, we end up with
a memory leak, as soon as we leave the function, and with
the global pointer conf set to NULL, leading to a segfault
as soon as it is dereferenced.
I tested it by calling a reconfigure and making the first
allocation in load_config fail but any failure in load_config
would do.
From a user perspective the CLI reports "fail".
If something like this should happen there are at least 2 possible
scenarios:
1) If a second immediate reconfigure succeeds, the conf now is fine but
the leak stays
2) If the previous point does not happen, any command trying to access
"conf" would fail. On my test box a "show conf" segfaulted.
The fix is simple but in case of failure at least the previous
conf is kept in memory without leaks or segfaluts
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
---
multipathd/main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/multipathd/main.c b/multipathd/main.c
index 04f6d02..f83c849 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1551,6 +1551,8 @@ reconfigure (struct vectors * vecs)
configure(vecs, 1);
free_config(old);
retval = 0;
+ } else {
+ conf = old;
}
running_state = DAEMON_RUNNING;
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] multipath: fix memory leak and segfault in reconfigure
2016-02-11 19:30 [PATCH 1/1] multipath: fix memory leak and segfault in reconfigure Germano Percossi
@ 2016-02-23 15:17 ` Germano Percossi
2016-04-18 10:42 ` Germano Percossi
1 sibling, 0 replies; 4+ messages in thread
From: Germano Percossi @ 2016-02-23 15:17 UTC (permalink / raw)
To: dm-devel
Ping
On 02/11/2016 07:30 PM, Germano Percossi wrote:
> Within the reconfigure function, the global pointer conf is
> stored in a local variable and then assigned NULL.
> If load_config should fail, for any reason, we end up with
> a memory leak, as soon as we leave the function, and with
> the global pointer conf set to NULL, leading to a segfault
> as soon as it is dereferenced.
>
> I tested it by calling a reconfigure and making the first
> allocation in load_config fail but any failure in load_config
> would do.
>>From a user perspective the CLI reports "fail".
>
> If something like this should happen there are at least 2 possible
> scenarios:
>
> 1) If a second immediate reconfigure succeeds, the conf now is fine but
> the leak stays
> 2) If the previous point does not happen, any command trying to access
> "conf" would fail. On my test box a "show conf" segfaulted.
>
> The fix is simple but in case of failure at least the previous
> conf is kept in memory without leaks or segfaluts
>
> Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
> ---
> multipathd/main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 04f6d02..f83c849 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1551,6 +1551,8 @@ reconfigure (struct vectors * vecs)
> configure(vecs, 1);
> free_config(old);
> retval = 0;
> + } else {
> + conf = old;
> }
>
> running_state = DAEMON_RUNNING;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] multipath: fix memory leak and segfault in reconfigure
2016-02-11 19:30 [PATCH 1/1] multipath: fix memory leak and segfault in reconfigure Germano Percossi
2016-02-23 15:17 ` Germano Percossi
@ 2016-04-18 10:42 ` Germano Percossi
2016-04-18 11:12 ` Christophe Varoqui
1 sibling, 1 reply; 4+ messages in thread
From: Germano Percossi @ 2016-04-18 10:42 UTC (permalink / raw)
To: christophe.varoqui; +Cc: dm-devel
Hi Christophe,
Is there a specific reason this silly patch is not getting
through?
It seems a no-brainer to me but probably I am missing something.
Thanks,
Germano
On 02/11/2016 07:30 PM, Germano Percossi wrote:
> Within the reconfigure function, the global pointer conf is
> stored in a local variable and then assigned NULL.
> If load_config should fail, for any reason, we end up with
> a memory leak, as soon as we leave the function, and with
> the global pointer conf set to NULL, leading to a segfault
> as soon as it is dereferenced.
>
> I tested it by calling a reconfigure and making the first
> allocation in load_config fail but any failure in load_config
> would do.
>>From a user perspective the CLI reports "fail".
>
> If something like this should happen there are at least 2 possible
> scenarios:
>
> 1) If a second immediate reconfigure succeeds, the conf now is fine but
> the leak stays
> 2) If the previous point does not happen, any command trying to access
> "conf" would fail. On my test box a "show conf" segfaulted.
>
> The fix is simple but in case of failure at least the previous
> conf is kept in memory without leaks or segfaluts
>
> Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
> ---
> multipathd/main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 04f6d02..f83c849 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1551,6 +1551,8 @@ reconfigure (struct vectors * vecs)
> configure(vecs, 1);
> free_config(old);
> retval = 0;
> + } else {
> + conf = old;
> }
>
> running_state = DAEMON_RUNNING;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] multipath: fix memory leak and segfault in reconfigure
2016-04-18 10:42 ` Germano Percossi
@ 2016-04-18 11:12 ` Christophe Varoqui
0 siblings, 0 replies; 4+ messages in thread
From: Christophe Varoqui @ 2016-04-18 11:12 UTC (permalink / raw)
To: Germano Percossi; +Cc: device-mapper development
[-- Attachment #1.1: Type: text/plain, Size: 2031 bytes --]
Hi Germano,
no reason at all ... it just missed my attention, twice. Sorry.
This patch is now merged.
Thanks,
Christophe Varoqui
www.opensvc.com
On Mon, Apr 18, 2016 at 12:42 PM, Germano Percossi <
germano.percossi@citrix.com> wrote:
> Hi Christophe,
>
> Is there a specific reason this silly patch is not getting
> through?
>
> It seems a no-brainer to me but probably I am missing something.
>
> Thanks,
> Germano
>
> On 02/11/2016 07:30 PM, Germano Percossi wrote:
>
>> Within the reconfigure function, the global pointer conf is
>> stored in a local variable and then assigned NULL.
>> If load_config should fail, for any reason, we end up with
>> a memory leak, as soon as we leave the function, and with
>> the global pointer conf set to NULL, leading to a segfault
>> as soon as it is dereferenced.
>>
>> I tested it by calling a reconfigure and making the first
>> allocation in load_config fail but any failure in load_config
>> would do.
>>
>>> From a user perspective the CLI reports "fail".
>>>
>>
>> If something like this should happen there are at least 2 possible
>> scenarios:
>>
>> 1) If a second immediate reconfigure succeeds, the conf now is fine but
>> the leak stays
>> 2) If the previous point does not happen, any command trying to access
>> "conf" would fail. On my test box a "show conf" segfaulted.
>>
>> The fix is simple but in case of failure at least the previous
>> conf is kept in memory without leaks or segfaluts
>>
>> Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
>> ---
>> multipathd/main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/multipathd/main.c b/multipathd/main.c
>> index 04f6d02..f83c849 100644
>> --- a/multipathd/main.c
>> +++ b/multipathd/main.c
>> @@ -1551,6 +1551,8 @@ reconfigure (struct vectors * vecs)
>> configure(vecs, 1);
>> free_config(old);
>> retval = 0;
>> + } else {
>> + conf = old;
>> }
>>
>> running_state = DAEMON_RUNNING;
>>
>>
[-- Attachment #1.2: Type: text/html, Size: 3031 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-18 11:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-11 19:30 [PATCH 1/1] multipath: fix memory leak and segfault in reconfigure Germano Percossi
2016-02-23 15:17 ` Germano Percossi
2016-04-18 10:42 ` Germano Percossi
2016-04-18 11:12 ` Christophe Varoqui
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.