Hi Mat,
On Wed, Apr 25, 2012 at 04:36:17PM -0700, Mat Martineau wrote:
> The ERTM and streaming mode transmit queue must only be accessed while
> the L2CAP channel lock is held. Adding this lock ensures that
> multiple threads cannot simultaneously manipulate the queue when
> sending and receiving concurrently.
I think the lock shall be added in l2cap_sock_sendmsg like:
lock()
err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
unlock()
I actually use this l2cap_chan_send in A2MP code already because it gets
locked in l2cap_data_channel.
Best regards
Andrei Emeltchenko
>
> Signed-off-by: Mat Martineau <mathewm@xxxxxxxxxxxxxx>
> ---
> net/bluetooth/l2cap_core.c | 40 ++++++++++++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 6aefeaa..8ed6a74 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2035,26 +2035,35 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> u32 control;
> int err;
>
> + l2cap_chan_hold(chan);
> +
> /* Connectionless channel */
> if (chan->chan_type == L2CAP_CHAN_CONN_LESS) {
> skb = l2cap_create_connless_pdu(chan, msg, len, priority);
> - if (IS_ERR(skb))
> - return PTR_ERR(skb);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + goto done;
> + }
>
> l2cap_do_send(chan, skb);
> - return len;
> + err = len;
> + goto done;
> }
>
> switch (chan->mode) {
> case L2CAP_MODE_BASIC:
> /* Check outgoing MTU */
> - if (len > chan->omtu)
> - return -EMSGSIZE;
> + if (len > chan->omtu) {
> + err = -EMSGSIZE;
> + break;
> + }
>
> /* Create a basic PDU */
> skb = l2cap_create_basic_pdu(chan, msg, len, priority);
> - if (IS_ERR(skb))
> - return PTR_ERR(skb);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + break;
> + }
>
> l2cap_do_send(chan, skb);
> err = len;
> @@ -2067,30 +2076,38 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> control = __set_ctrl_sar(chan, L2CAP_SAR_UNSEGMENTED);
> skb = l2cap_create_iframe_pdu(chan, msg, len, control,
> 0);
> - if (IS_ERR(skb))
> - return PTR_ERR(skb);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + break;
> + }
>
> + l2cap_chan_lock(chan);
> __skb_queue_tail(&chan->tx_q, skb);
>
> if (chan->tx_send_head == NULL)
> chan->tx_send_head = skb;
>
> + l2cap_chan_unlock(chan);
> } else {
> /* Segment SDU into multiples PDUs */
> err = l2cap_sar_segment_sdu(chan, msg, len);
> if (err < 0)
> - return err;
> + break;
> }
>
> if (chan->mode == L2CAP_MODE_STREAMING) {
> + l2cap_chan_lock(chan);
> l2cap_streaming_send(chan);
> + l2cap_chan_unlock(chan);
> err = len;
> break;
> }
>
> + l2cap_chan_lock(chan);
> if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state) &&
> test_bit(CONN_WAIT_F, &chan->conn_state)) {
> err = len;
> + l2cap_chan_unlock(chan);
> break;
> }
>
> @@ -2098,6 +2115,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> if (err >= 0)
> err = len;
>
> + l2cap_chan_unlock(chan);
> break;
>
> default:
> @@ -2105,6 +2123,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> err = -EBADFD;
> }
>
> +done:
> + l2cap_chan_put(chan);
> return err;
> }
>
> --
> 1.7.10
>
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
