aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorPauli Virtanen <pav@iki.fi>2024-03-16 20:36:38 +0200
committerLuiz Augusto von Dentz <luiz.von.dentz@intel.com>2024-03-20 09:15:35 +0000
commitc04b96dda5ce1bbb07a72b7ffa5ad1786ccffe47 (patch)
tree67bf08cc125e317c2005774dc4892d93c63dafed
parent8060d1208673826665b7297c27aa75003521b52a (diff)
a2dp: fix setup->err use-after-free
setup->err is set to values that either are on stack of avdtp.c routines, obtained from callbacks, or allocated on heap. This is inconsistent, and use-after-free in some cases. Fix by always allocating setup->err ourselves, copying any values obtained from callbacks. Add setup_error_set/init and do all setup->err manipulation via them. Fixes crash: ==994225==ERROR: AddressSanitizer: stack-use-after-return READ of size 1 at 0x7f15ee5189c0 thread T0 #0 0x445724 in avdtp_error_category profiles/audio/avdtp.c:657 #1 0x41e59e in error_to_errno profiles/audio/a2dp.c:303 #2 0x42bb23 in a2dp_reconfigure profiles/audio/a2dp.c:1336 #3 0x7f15f1512798 in g_timeout_dispatch ... Address 0x7f15ee5189c0 is located in stack of thread T0 at offset 64 in frame #0 0x466b76 in avdtp_parse_rej profiles/audio/avdtp.c:3056 This frame has 2 object(s): [48, 49) 'acp_seid' (line 3058) [64, 72) 'err' (line 3057) <== Memory access at offset 64 is inside this variable
-rw-r--r--profiles/audio/a2dp.c68
1 files changed, 39 insertions, 29 deletions
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index b43161a136..a3c294bc30 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -222,6 +222,7 @@ static void setup_free(struct a2dp_setup *s)
avdtp_unref(s->session);
g_slist_free_full(s->cb, g_free);
g_slist_free_full(s->caps, g_free);
+ g_free(s->err);
g_free(s);
}
@@ -270,17 +271,34 @@ static void setup_cb_free(struct a2dp_setup_cb *cb)
g_free(cb);
}
+static void setup_error_set(struct a2dp_setup *setup, struct avdtp_error *err)
+{
+ if (!err) {
+ g_free(setup->err);
+ setup->err = NULL;
+ } else {
+ if (!setup->err)
+ setup->err = g_new0(struct avdtp_error, 1);
+ memcpy(setup->err, err, sizeof(struct avdtp_error));
+ }
+}
+
+static void setup_error_init(struct a2dp_setup *setup, uint8_t type, int id)
+{
+ struct avdtp_error err;
+
+ avdtp_error_init(&err, type, id);
+ setup_error_set(setup, &err);
+}
+
static void finalize_setup_errno(struct a2dp_setup *s, int err,
GSourceFunc cb1, ...)
{
GSourceFunc finalize;
va_list args;
- struct avdtp_error avdtp_err;
- if (err < 0) {
- avdtp_error_init(&avdtp_err, AVDTP_ERRNO, -err);
- s->err = &avdtp_err;
- }
+ if (err < 0)
+ setup_error_init(s, AVDTP_ERRNO, -err);
va_start(args, cb1);
finalize = cb1;
@@ -576,10 +594,7 @@ done:
finalize_config(setup);
- if (setup->err) {
- g_free(setup->err);
- setup->err = NULL;
- }
+ setup_error_set(setup, NULL);
setup_unref(setup);
@@ -588,11 +603,9 @@ done:
static void endpoint_setconf_cb(struct a2dp_setup *setup, gboolean ret)
{
- if (ret == FALSE) {
- setup->err = g_new(struct avdtp_error, 1);
- avdtp_error_init(setup->err, AVDTP_MEDIA_CODEC,
+ if (ret == FALSE)
+ setup_error_init(setup, AVDTP_MEDIA_CODEC,
AVDTP_UNSUPPORTED_CONFIGURATION);
- }
auto_config(setup);
setup_unref(setup);
@@ -671,8 +684,7 @@ static gboolean endpoint_setconf_ind(struct avdtp *session,
if (cap->category == AVDTP_DELAY_REPORTING &&
!a2dp_sep->delay_reporting) {
- setup->err = g_new(struct avdtp_error, 1);
- avdtp_error_init(setup->err, AVDTP_DELAY_REPORTING,
+ setup_error_init(setup, AVDTP_DELAY_REPORTING,
AVDTP_UNSUPPORTED_CONFIGURATION);
goto done;
}
@@ -683,8 +695,7 @@ static gboolean endpoint_setconf_ind(struct avdtp *session,
codec = (struct avdtp_media_codec_capability *) cap->data;
if (codec->media_codec_type != a2dp_sep->codec) {
- setup->err = g_new(struct avdtp_error, 1);
- avdtp_error_init(setup->err, AVDTP_MEDIA_CODEC,
+ setup_error_init(setup, AVDTP_MEDIA_CODEC,
AVDTP_UNSUPPORTED_CONFIGURATION);
goto done;
}
@@ -704,10 +715,9 @@ static gboolean endpoint_setconf_ind(struct avdtp *session,
return TRUE;
}
- setup_unref(setup);
- setup->err = g_new(struct avdtp_error, 1);
- avdtp_error_init(setup->err, AVDTP_MEDIA_CODEC,
+ setup_error_init(setup, AVDTP_MEDIA_CODEC,
AVDTP_UNSUPPORTED_CONFIGURATION);
+ setup_unref(setup);
break;
}
@@ -886,7 +896,7 @@ static void invalidate_remote_cache(struct a2dp_setup *setup,
/* Set error to -EAGAIN so the likes of policy plugin can
* reattempt to connect.
*/
- avdtp_error_init(setup->err, AVDTP_ERRNO, -EAGAIN);
+ setup_error_init(setup, AVDTP_ERRNO, -EAGAIN);
}
}
@@ -910,10 +920,10 @@ static void setconf_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (err) {
if (setup) {
setup_ref(setup);
- setup->err = err;
+ setup_error_set(setup, err);
invalidate_remote_cache(setup, err);
finalize_config(setup);
- setup->err = NULL;
+ setup_error_set(setup, NULL);
setup_unref(setup);
}
@@ -1116,7 +1126,7 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (err) {
setup->stream = NULL;
- setup->err = err;
+ setup_error_set(setup, err);
if (setup->start)
finalize_resume(setup);
} else if (setup->chan)
@@ -1191,7 +1201,7 @@ static void start_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (err) {
setup->stream = NULL;
- setup->err = err;
+ setup_error_set(setup, err);
}
finalize_resume(setup);
@@ -1270,7 +1280,7 @@ static void suspend_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (err) {
setup->stream = NULL;
- setup->err = err;
+ setup_error_set(setup, err);
}
finalize_suspend(setup);
@@ -1418,7 +1428,7 @@ static void close_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (err) {
setup->stream = NULL;
- setup->err = err;
+ setup_error_set(setup, err);
finalize_config(setup);
return;
}
@@ -1528,7 +1538,7 @@ static void reconf_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (err) {
setup->stream = NULL;
- setup->err = err;
+ setup_error_set(setup, err);
}
finalize_config(setup);
@@ -2885,7 +2895,7 @@ static void discover_cb(struct avdtp *session, GSList *seps,
setup->seps = seps;
if (err)
- setup->err = err;
+ setup_error_set(setup, err);
if (!err) {
g_slist_foreach(seps, foreach_register_remote_sep, setup->chan);