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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.