2013-01-21 13:33:29

by Syam Sidhardhan

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

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



2013-01-22 12:04:14

by Luiz Augusto von Dentz

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

Hi Syam,

On Mon, Jan 21, 2013 at 3:33 PM, Syam Sidhardhan <[email protected]> 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

2013-01-21 13:33:31

by Syam Sidhardhan

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

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


2013-01-21 13:33:30

by Syam Sidhardhan

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

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