From: Peter Hurley <peter@hurleysoftware.com>
To: linux-bluetooth <linux-bluetooth@vger.kernel.org>
Subject: [PATCH] Audio: fix race condition when starting a stream
Date: Thu, 7 Jul 2011 10:51:34 -0400 [thread overview]
Message-ID: <1310050294.5639.1.camel@THOR> (raw)
The AVDTP spec allows for a race condition between remote and local
device when issuing an AVDTP_START cmd on a stream in the OPEN state.
However, the internal state must continue to be consistent. For example,
suppose that avdtp_start() has been called while in the OPEN state and
a AVDTP_START cmd is sent. Now before we have received a response (and
thus entered the STREAMING state), we *receive* a START cmd. Prior to
this fix, since the sep is still in the OPEN state, we would accept
the new START cmd. This will leads us to send both a Start_Ind and
Start_Cfm - not good.
Now, we track this transitional state (starting == TRUE).
NB - 'starting' is only in a valid state while the sep is in the
OPEN state. 'starting' is reset when we return to the OPEN state.
---
audio/avdtp.c | 29 +++++++++++++++++++++++++----
1 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/audio/avdtp.c b/audio/avdtp.c
index 44df636..c12f5a3 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -378,6 +378,7 @@ struct avdtp_stream {
guint idle_timer;
gboolean delay_reporting;
uint16_t delay; /* AVDTP 1.3 Delay Reporting feature */
+ gboolean starting; /* only valid while sep state == OPEN */
};
/* Structure describing an AVDTP connection between two devices */
@@ -1068,6 +1069,7 @@ static void avdtp_sep_set_state(struct avdtp *session,
avdtp_delay_report(session, stream, stream->delay);
break;
case AVDTP_STATE_OPEN:
+ stream->starting = FALSE;
if (old_state > AVDTP_STATE_OPEN && session->auto_dc)
stream->idle_timer = g_timeout_add_seconds(STREAM_TIMEOUT,
stream_timeout,
@@ -1712,10 +1714,13 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
stream = sep->stream;
- if (sep->state != AVDTP_STATE_OPEN) {
+ /* Also reject start cmd if we already initiated start */
+ if (sep->state != AVDTP_STATE_OPEN ||
+ stream->starting == TRUE) {
err = AVDTP_BAD_STATE;
goto failed;
}
+ stream->starting = TRUE;
if (sep->ind && sep->ind->start) {
if (!sep->ind->start(session, sep, stream, &err,
@@ -1730,6 +1735,7 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
AVDTP_START, NULL, 0);
failed:
+ DBG("Rejecting (%d)", err);
memset(&rej, 0, sizeof(rej));
rej.acp_seid = failed_seid;
rej.error = err;
@@ -2584,9 +2590,12 @@ static int cancel_request(struct avdtp *session, int err)
break;
case AVDTP_START:
error("Start: %s (%d)", strerror(err), err);
- if (lsep && lsep->cfm && lsep->cfm->start)
+ if (lsep && lsep->cfm && lsep->cfm->start) {
lsep->cfm->start(session, lsep, stream, &averr,
lsep->user_data);
+ if (stream)
+ stream->starting = FALSE;
+ }
break;
case AVDTP_SUSPEND:
error("Suspend: %s (%d)", strerror(err), err);
@@ -3092,9 +3101,11 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
return FALSE;
error("START request rejected: %s (%d)",
avdtp_strerror(&err), err.err.error_code);
- if (sep && sep->cfm && sep->cfm->start)
+ if (sep && sep->cfm && sep->cfm->start) {
sep->cfm->start(session, sep, stream, &err,
sep->user_data);
+ stream->starting = FALSE;
+ }
return TRUE;
case AVDTP_SUSPEND:
if (!stream_rej_to_err(buf, size, &err, &acp_seid))
@@ -3552,6 +3563,7 @@ int avdtp_open(struct avdtp *session, struct avdtp_stream *stream)
int avdtp_start(struct avdtp *session, struct avdtp_stream *stream)
{
struct start_req req;
+ int ret;
if (!g_slist_find(session->streams, stream))
return -EINVAL;
@@ -3564,11 +3576,20 @@ int avdtp_start(struct avdtp *session, struct avdtp_stream *stream)
return -EINVAL;
}
+ if (stream->starting == TRUE) {
+ DBG("stream already started");
+ return -EINVAL;
+ }
+
memset(&req, 0, sizeof(req));
req.first_seid.seid = stream->rseid;
- return send_request(session, FALSE, stream, AVDTP_START,
+ ret = send_request(session, FALSE, stream, AVDTP_START,
&req, sizeof(req));
+ if (ret == 0)
+ stream->starting = TRUE;
+
+ return ret;
}
int avdtp_close(struct avdtp *session, struct avdtp_stream *stream,
--
1.7.4.1
next reply other threads:[~2011-07-07 14:51 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-07 14:51 Peter Hurley [this message]
2011-07-10 7:55 ` [PATCH] Audio: fix race condition when starting a stream Johan Hedberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1310050294.5639.1.camel@THOR \
--to=peter@hurleysoftware.com \
--cc=linux-bluetooth@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox