[PATCH 1/3] health: Fix possible use after free

[PATCH 1/3] health: Fix possible use after free

[Syam Sidhardhan]

A pointer to freed memory is dereferenced if we call function
hdp_get_dcpsm_cb() with out any earlier reference.
---
 profiles/health/hdp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index c15f06a..a42ca48 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -542,9 +542,9 @@ static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
 					hdp_tmp_dc_data_destroy, &gerr))
 		return;
 
-	hdp_tmp_dc_data_unref(hdp_conn);
 	hdp_conn->cb(hdp_chann->mdl, err, hdp_conn);
 	g_error_free(gerr);
+	hdp_tmp_dc_data_unref(hdp_conn);
 }
 
 static void device_reconnect_mdl_cb(struct mcap_mdl *mdl, GError *err,
-- 
1.7.9.5

[PATCH 2/3] a2dp: Fix invalid memory access during suspend_ind()

[Syam Sidhardhan]

There is a possible invalid memory access during suspend_ind().
We should terminate the variable arguments with NULL.
---
 profiles/audio/a2dp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 46f41a6..3c546d9 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -801,7 +801,7 @@ static gboolean suspend_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 	if (start_err < 0 && start_err != -EINPROGRESS) {
 		error("avdtp_start: %s (%d)", strerror(-start_err),
 								-start_err);
-		finalize_setup_errno(setup, start_err, finalize_resume);
+		finalize_setup_errno(setup, start_err, finalize_resume, NULL);
 	}
 
 	return TRUE;
-- 
1.7.9.5

[PATCH 3/3] a2dp: Fix invalid memory access during abort_ind()

[Syam Sidhardhan]

There is an invalid memory access w.r.t to the callback
during the Abort_Ind finalize_setup_errno().

We should terminate the variable arguments with NULL.

Log:
bluetoothd[3353]: audio/avdtp.c:avdtp_parse_cmd() Received ABORT_CMD
bluetoothd[3353]: audio/a2dp.c:abort_ind() Source 0xb6f5ecc8: Abort_Ind
bluetoothd[3353]: audio/a2dp.c:setup_ref() 0xb6f63200: ref=2
bluetoothd[3353]: audio/transport.c:media_transport_remove() Transport
 /org/bluez/3353/hci0/dev_BC_47_60_F5_88_89/fd1 Owner :1.0
bluetoothd[3353]: audio/transport.c:media_transport_release() Transport
 /org/bluez/3353/hci0/dev_BC_47_60_F5_88_89/fd1: read lock released
bluetoothd[3353]: audio/transport.c:media_transport_release() Transport
 /org/bluez/3353/hci0/dev_BC_47_60_F5_88_89/fd1: write lock released
bluetoothd[3353]: audio/transport.c:media_request_reply() Request
 Acquire Reply Input/output error
bluetoothd[3353]: audio/transport.c:media_owner_free() Owner :1.0
bluetoothd[3353]: audio/transport.c:media_owner_remove() Owner :1.0 Request Acquire
bluetoothd[3353]: audio/a2dp.c:a2dp_sep_unlock() SEP 0xb6f5ecc8 unlocked
bluetoothd[3353]: audio/a2dp.c:setup_unref() 0xb6f63200: ref=1
[sys_assert]START of sighandler
[sys-assert]exepath = bluetoothd
[sys-assert]processname = bluetoothd
[sys_assert]this thread is main thread. pid=3353
[sys-assert]cs timestr 1358524835
bluetoothd[3353]: crashed [1358524835] processname=bluetoothd, pid=3353, tid=3353, signal=11
[sys-assert]start print_node_to_file
sighandler = 0xb6e8cfc9, g_sig_oldact[i] = (nil)
[sys_assert]END of sighandler
Segmentation fault (core dumped)
---
 profiles/audio/a2dp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 3c546d9..efb4178 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -957,7 +957,7 @@ static void abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 
 	finalize_setup_errno(setup, -ECONNRESET, finalize_suspend,
 							finalize_resume,
-							finalize_config);
+							finalize_config, NULL);
 
 	return;
 }
-- 
1.7.9.5

Re: [PATCH 1/3] health: Fix possible use after free

[Luiz Augusto von Dentz]

Hi Syam,

On Mon, Jan 21, 2013 at 3:33 PM, Syam Sidhardhan <s.syam@samsung.com> wrote:
> A pointer to freed memory is dereferenced if we call function
> hdp_get_dcpsm_cb() with out any earlier reference.
> ---
>  profiles/health/hdp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
> index c15f06a..a42ca48 100644
> --- a/profiles/health/hdp.c
> +++ b/profiles/health/hdp.c
> @@ -542,9 +542,9 @@ static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
>                                         hdp_tmp_dc_data_destroy, &gerr))
>                 return;
>
> -       hdp_tmp_dc_data_unref(hdp_conn);
>         hdp_conn->cb(hdp_chann->mdl, err, hdp_conn);
>         g_error_free(gerr);
> +       hdp_tmp_dc_data_unref(hdp_conn);
>  }
>
>  static void device_reconnect_mdl_cb(struct mcap_mdl *mdl, GError *err,
> --
> 1.7.9.5

All 3 patches are now upstream, thanks.


--
Luiz Augusto von Dentz