Hi Gustavo, On Wed, May 11, 2011 at 8:09 PM, Gustavo F. Padovan <padovan@xxxxxxxxxxxxxx> wrote: > Hi Luiz, > > * Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> [2011-05-05 17:50:53 +0300]: > >> Hi Gustavo, >> >> On Mon, Apr 18, 2011 at 8:56 PM, Gustavo F. Padovan >> <padovan@xxxxxxxxxxxxxx> wrote: >> > * Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> [2011-04-17 20:26:53 +0300]: >> > >> >> Hi Gustavo, >> >> >> >> On Fri, Apr 15, 2011 at 9:58 PM, Gustavo F. Padovan >> >> <padovan@xxxxxxxxxxxxxx> wrote: >> >> > Hi Luiz, >> >> > >> >> > * Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> [2011-04-08 17:10:41 +0300]: >> >> > >> >> >> From: Luiz Augusto von Dentz <luiz.dentz-von@xxxxxxxxx> >> >> >> >> >> >> shutdown should wait for SCO link to be properly disconnected before >> >> >> detroying the socket, otherwise an application using the socket may >> >> >> assume link is properly disconnected before it really happens which >> >> >> can be a problem when e.g synchronizing profile switch. >> >> >> >> >> >> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@xxxxxxxxx> >> >> > >> >> > I applied it, but in bluetooth-next. Let's see its behaviour there and if no >> >> > problems show up we can move it to bluetooth-2.6 >> >> >> >> I tested this against Nokia BH-504 and Sony Ericsson W600, both have >> >> problem when switching from hfp to a2dp where the avdtp start is sent >> >> before SCO is fully disconnected, this patch fixes with those >> >> headsets. >> > >> > Ok, I also pushed it to bluetooth-2.6. >> >> >> Apparently this cause a regression, since we set conn to NULL but an >> application may not wait for shutdown to complete and call >> close/release which will cause sco_chan_del to be called which destroy >> the socket without resetting conn->sk to NULL so on disconn_cfm it >> will access invalid memory. >> >> To fix this what about the following: >> >> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c >> index 94954c7..cb4fb78 100644 >> --- a/net/bluetooth/sco.c >> +++ b/net/bluetooth/sco.c >> @@ -373,7 +373,7 @@ static void __sco_sock_close(struct sock *sk) >> sk->sk_state = BT_DISCONN; >> sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT); >> hci_conn_put(sco_pi(sk)->conn->hcon); >> - sco_pi(sk)->conn = NULL; >> + sco_pi(sk)->conn->hcon = NULL; >> } else >> sco_chan_del(sk, ECONNRESET); >> break; >> @@ -828,7 +828,9 @@ static void sco_chan_del(struct sock *sk, int err) >> conn->sk = NULL; >> sco_pi(sk)->conn = NULL; >> sco_conn_unlock(conn); >> - hci_conn_put(conn->hcon); >> + >> + if (conn->hcon) >> + hci_conn_put(conn->hcon); > > I think first we need to revert the patch on linus' tree. There isn't time to > a proper fix and test. It may have introduced other bugs too. I don't wanna > take this risk. Ville and I tested the suggested changes and see no regression anymore, so I can either create a new patch or update the other in case we revert it. -- Luiz Augusto von Dentz Computer Engineer -- 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
