[PATCH BlueZ 1/1] scr:Set property mode failed,memory leak

[PATCH BlueZ 1/1] scr:Set property mode failed,memory leak

[chengbo]

This patch will fix a memory leak,when set property mode,
it will creat a request,if failed,the data's memory do not free
---
 src/adapter.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5e896a9f0..3d07921a7 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2917,9 +2917,10 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
 	data->id = id;
 
 	if (mgmt_send(adapter->mgmt, opcode, adapter->dev_id, len, param,
-			property_set_mode_complete, data, g_free) > 0)
+			property_set_mode_complete, data, g_free) > 0) {
+		g_free(data);
 		return;
-
+	}
 	g_free(data);
 
 failed:
-- 
2.20.1

Re: [PATCH BlueZ 1/1] scr:Set property mode failed,memory leak

[Luiz Augusto von Dentz]

Hi Chengbo,

On Sun, Aug 16, 2020 at 7:57 PM chengbo <515672508@qq.com> wrote:
>
> This patch will fix a memory leak,when set property mode,
> it will creat a request,if failed,the data's memory do not free
> ---
>  src/adapter.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 5e896a9f0..3d07921a7 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -2917,9 +2917,10 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
>         data->id = id;
>
>         if (mgmt_send(adapter->mgmt, opcode, adapter->dev_id, len, param,
> -                       property_set_mode_complete, data, g_free) > 0)
> +                       property_set_mode_complete, data, g_free) > 0) {
> +               g_free(data);
>                 return;
> -
> +       }

This is actually the other way around, if it fails then 0 is returned
and g_free is called, so this would cause a double free as g_free
would be called on destroy.

>         g_free(data);
>
>  failed:
> --
> 2.20.1
>
>
>


-- 
Luiz Augusto von Dentz

[PATCH] scr:Set property mode failed,memory leak

[chengbo]

This patch will fix a memory leak,when set property mode,
it will creat a request,if failed,the data's memory do not free
---
 src/adapter.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5e896a9f0..3d07921a7 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2917,9 +2917,10 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
 	data->id = id;
 
 	if (mgmt_send(adapter->mgmt, opcode, adapter->dev_id, len, param,
-			property_set_mode_complete, data, g_free) > 0)
+			property_set_mode_complete, data, g_free) > 0) {
+		g_free(data);
 		return;
-
+	}

In the original code,if mgmt_send fails then 0 is returned,then this function return directly,do not free data.
Therefore, you need to free data before executing return.

 	g_free(data);
 
 failed:
-- 
2.20.1

Re: [PATCH] scr:Set property mode failed,memory leak

[Luiz Augusto von Dentz]

Hi Chengbo,

On Mon, Aug 17, 2020 at 6:24 PM chengbo <515672508@qq.com> wrote:
>
> This patch will fix a memory leak,when set property mode,
> it will creat a request,if failed,the data's memory do not free
> ---
>  src/adapter.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 5e896a9f0..3d07921a7 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -2917,9 +2917,10 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
>         data->id = id;
>
>         if (mgmt_send(adapter->mgmt, opcode, adapter->dev_id, len, param,
> -                       property_set_mode_complete, data, g_free) > 0)
> +                       property_set_mode_complete, data, g_free) > 0) {
> +               g_free(data);
>                 return;
> -
> +       }
>
> In the original code,if mgmt_send fails then 0 is returned,then this function return directly,do not free data.
> Therefore, you need to free data before executing return.

Nope, if (0 > 0) will evaluate to false so it will continue and the
code below will be executed, the if branch is testing for success case
not the opposite and that is why there is no goto failed either.

>
>         g_free(data);
>
>  failed:
> --
> 2.20.1
>
>
>


-- 
Luiz Augusto von Dentz