* [PATCH 0/2] Fix usage of sk_sndtimeo value in l2cap @ 2011-11-10 9:37 Andrzej Kaczmarek 2011-11-10 9:37 ` [PATCH 1/2] Buetooth: Revert "Fixed wrong L2CAP Sock timer value" Andrzej Kaczmarek 2011-11-10 9:37 ` [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value Andrzej Kaczmarek 0 siblings, 2 replies; 8+ messages in thread From: Andrzej Kaczmarek @ 2011-11-10 9:37 UTC (permalink / raw) To: linux-bluetooth Cc: kanak.gupta, ulrik.lauren, henrik.possung, Andrzej Kaczmarek Hi, It seems sk_sndtimeo is initialized incorrectly in l2cap_sock.c to miliseconds, this will cause invalid value to be set or read using SO_SNDTIMEO sockopt since it is assumed this value is specified in jiffies. Initial value of sk_sndtimeo was changed in 6be6b11f006840ba7d8d4b and I belive this was because of regression introduced by commit 942ecc9c4643db5ce071562e0a23f99464d6b461. My two patches revert mentioned commit and solve issue in proper way. Regression introduced by 942ecc9c4643db5ce071562e0a23f99464d6b461 is already fixed by other commit. Andrzej Kaczmarek (2): Buetooth: Revert "Fixed wrong L2CAP Sock timer value" Bluetooth: Fix usage of sk_sndtimeo value net/bluetooth/l2cap_core.c | 8 ++++---- net/bluetooth/l2cap_sock.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) -- on behalf of ST-Ericsson ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] Buetooth: Revert "Fixed wrong L2CAP Sock timer value" 2011-11-10 9:37 [PATCH 0/2] Fix usage of sk_sndtimeo value in l2cap Andrzej Kaczmarek @ 2011-11-10 9:37 ` Andrzej Kaczmarek 2011-11-10 9:37 ` [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value Andrzej Kaczmarek 1 sibling, 0 replies; 8+ messages in thread From: Andrzej Kaczmarek @ 2011-11-10 9:37 UTC (permalink / raw) To: linux-bluetooth Cc: kanak.gupta, ulrik.lauren, henrik.possung, Andrzej Kaczmarek This reverts commit 6be6b11f006840ba7d8d4b959b3fa0c522f8468a. Commit changed sk_sndtimeo value unit to miliseconds while jiffies should be used. This will cause invalid behaviour if timeout is read or set via SO_SNDTIMEO sockopt which uses jiffies. Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com> --- net/bluetooth/l2cap_sock.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 664762e..ae7dbe9 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1039,7 +1039,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int p INIT_LIST_HEAD(&bt_sk(sk)->accept_q); sk->sk_destruct = l2cap_sock_destruct; - sk->sk_sndtimeo = L2CAP_CONN_TIMEOUT; + sk->sk_sndtimeo = msecs_to_jiffies(L2CAP_CONN_TIMEOUT); sock_reset_flag(sk, SOCK_ZAPPED); -- on behalf of ST-Ericsson ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value 2011-11-10 9:37 [PATCH 0/2] Fix usage of sk_sndtimeo value in l2cap Andrzej Kaczmarek 2011-11-10 9:37 ` [PATCH 1/2] Buetooth: Revert "Fixed wrong L2CAP Sock timer value" Andrzej Kaczmarek @ 2011-11-10 9:37 ` Andrzej Kaczmarek 2011-11-10 9:57 ` Andrei Emeltchenko 2011-12-18 23:31 ` Gustavo Padovan 1 sibling, 2 replies; 8+ messages in thread From: Andrzej Kaczmarek @ 2011-11-10 9:37 UTC (permalink / raw) To: linux-bluetooth Cc: kanak.gupta, ulrik.lauren, henrik.possung, Andrzej Kaczmarek sk_sndtimeo timeout value is specified in jiffes and should be converted to miliseconds when used as input to __set_chan_timer. Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com> --- net/bluetooth/l2cap_core.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index f850684..3b0f807 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -446,7 +446,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason) if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED && conn->hcon->type == ACL_LINK) { __clear_chan_timer(chan); - __set_chan_timer(chan, sk->sk_sndtimeo); + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); l2cap_send_disconn_req(conn, chan, reason); } else l2cap_chan_del(chan, reason); @@ -899,7 +899,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) __l2cap_chan_add(conn, chan); - __set_chan_timer(chan, sk->sk_sndtimeo); + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); l2cap_state_change(chan, BT_CONNECTED); parent->sk_data_ready(parent, 0); @@ -1176,7 +1176,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan) l2cap_chan_add(conn, chan); l2cap_state_change(chan, BT_CONNECT); - __set_chan_timer(chan, sk->sk_sndtimeo); + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); if (hcon->state == BT_CONNECTED) { if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) { @@ -2601,7 +2601,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd dcid = chan->scid; - __set_chan_timer(chan, sk->sk_sndtimeo); + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); chan->ident = cmd->ident; -- on behalf of ST-Ericsson ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value 2011-11-10 9:37 ` [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value Andrzej Kaczmarek @ 2011-11-10 9:57 ` Andrei Emeltchenko 2011-11-10 10:13 ` Andrzej Kaczmarek 2011-12-18 23:31 ` Gustavo Padovan 1 sibling, 1 reply; 8+ messages in thread From: Andrei Emeltchenko @ 2011-11-10 9:57 UTC (permalink / raw) To: Andrzej Kaczmarek Cc: linux-bluetooth, kanak.gupta, ulrik.lauren, henrik.possung Hi Andrzej, On Thu, Nov 10, 2011 at 10:37:25AM +0100, Andrzej Kaczmarek wrote: > sk_sndtimeo timeout value is specified in jiffes and should be > converted to miliseconds when used as input to __set_chan_timer. > > Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com> > --- > net/bluetooth/l2cap_core.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index f850684..3b0f807 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -446,7 +446,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason) > if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED && > conn->hcon->type == ACL_LINK) { > __clear_chan_timer(chan); > - __set_chan_timer(chan, sk->sk_sndtimeo); > + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); Then __set_chan_timer do reverse conversion: mod_timer(timer, jiffies + msecs_to_jiffies(timeout)) Look ugly :-( Best regards Andrei Emeltchenko > l2cap_send_disconn_req(conn, chan, reason); > } else > l2cap_chan_del(chan, reason); > @@ -899,7 +899,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) > > __l2cap_chan_add(conn, chan); > > - __set_chan_timer(chan, sk->sk_sndtimeo); > + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); > > l2cap_state_change(chan, BT_CONNECTED); > parent->sk_data_ready(parent, 0); > @@ -1176,7 +1176,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan) > l2cap_chan_add(conn, chan); > > l2cap_state_change(chan, BT_CONNECT); > - __set_chan_timer(chan, sk->sk_sndtimeo); > + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); > > if (hcon->state == BT_CONNECTED) { > if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) { > @@ -2601,7 +2601,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > > dcid = chan->scid; > > - __set_chan_timer(chan, sk->sk_sndtimeo); > + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); > > chan->ident = cmd->ident; > > -- > on behalf of ST-Ericsson > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value 2011-11-10 9:57 ` Andrei Emeltchenko @ 2011-11-10 10:13 ` Andrzej Kaczmarek 2011-11-10 10:35 ` Andrei Emeltchenko 0 siblings, 1 reply; 8+ messages in thread From: Andrzej Kaczmarek @ 2011-11-10 10:13 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth, kanak.gupta, ulrik.lauren, henrik.possung Hi Andrei, On 10.11.2011 10:57, Andrei Emeltchenko wrote: > Hi Andrzej, > > On Thu, Nov 10, 2011 at 10:37:25AM +0100, Andrzej Kaczmarek wrote: >> sk_sndtimeo timeout value is specified in jiffes and should be >> converted to miliseconds when used as input to __set_chan_timer. >> >> Signed-off-by: Andrzej Kaczmarek<andrzej.kaczmarek@tieto.com> >> --- >> net/bluetooth/l2cap_core.c | 8 ++++---- >> 1 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >> index f850684..3b0f807 100644 >> --- a/net/bluetooth/l2cap_core.c >> +++ b/net/bluetooth/l2cap_core.c >> @@ -446,7 +446,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason) >> if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED&& >> conn->hcon->type == ACL_LINK) { >> __clear_chan_timer(chan); >> - __set_chan_timer(chan, sk->sk_sndtimeo); >> + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); > > Then __set_chan_timer do reverse conversion: > mod_timer(timer, jiffies + msecs_to_jiffies(timeout)) > > Look ugly :-( Well, I agree. But problem here is that __set_chan_timer has miliseconds as input and sk_sndtimeo has to be specified in jiffies thus ugly double conversion. But what about adding __set_chan_timer_jiffies macro and use it here? BR, Andrzej ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value 2011-11-10 10:13 ` Andrzej Kaczmarek @ 2011-11-10 10:35 ` Andrei Emeltchenko 2011-11-10 12:09 ` Andrzej Kaczmarek 0 siblings, 1 reply; 8+ messages in thread From: Andrei Emeltchenko @ 2011-11-10 10:35 UTC (permalink / raw) To: Andrzej Kaczmarek Cc: linux-bluetooth, kanak.gupta, ulrik.lauren, henrik.possung Hi Andrzej, On Thu, Nov 10, 2011 at 11:13:53AM +0100, Andrzej Kaczmarek wrote: > Hi Andrei, > > On 10.11.2011 10:57, Andrei Emeltchenko wrote: > >Hi Andrzej, > > > >On Thu, Nov 10, 2011 at 10:37:25AM +0100, Andrzej Kaczmarek wrote: > >>sk_sndtimeo timeout value is specified in jiffes and should be > >>converted to miliseconds when used as input to __set_chan_timer. > >> > >>Signed-off-by: Andrzej Kaczmarek<andrzej.kaczmarek@tieto.com> > >>--- > >> net/bluetooth/l2cap_core.c | 8 ++++---- > >> 1 files changed, 4 insertions(+), 4 deletions(-) > >> > >>diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > >>index f850684..3b0f807 100644 > >>--- a/net/bluetooth/l2cap_core.c > >>+++ b/net/bluetooth/l2cap_core.c > >>@@ -446,7 +446,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason) > >> if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED&& > >> conn->hcon->type == ACL_LINK) { > >> __clear_chan_timer(chan); > >>- __set_chan_timer(chan, sk->sk_sndtimeo); > >>+ __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); > > > >Then __set_chan_timer do reverse conversion: > >mod_timer(timer, jiffies + msecs_to_jiffies(timeout)) > > > >Look ugly :-( > > Well, I agree. But problem here is that __set_chan_timer has > miliseconds as input and sk_sndtimeo has to be specified in jiffies > thus ugly double conversion. Maybe we can just use jiffies in __set_chan_timer ? Best regards Andrei Emeltchenko > > But what about adding __set_chan_timer_jiffies macro and use it here? > > BR, > Andrzej ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value 2011-11-10 10:35 ` Andrei Emeltchenko @ 2011-11-10 12:09 ` Andrzej Kaczmarek 0 siblings, 0 replies; 8+ messages in thread From: Andrzej Kaczmarek @ 2011-11-10 12:09 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth, kanak.gupta, ulrik.lauren, henrik.possung Hi Andrei, On 10.11.2011 11:35, Andrei Emeltchenko wrote: > Hi Andrzej, > > On Thu, Nov 10, 2011 at 11:13:53AM +0100, Andrzej Kaczmarek wrote: >> Hi Andrei, >> >> On 10.11.2011 10:57, Andrei Emeltchenko wrote: >>> Hi Andrzej, >>> >>> On Thu, Nov 10, 2011 at 10:37:25AM +0100, Andrzej Kaczmarek wrote: >>>> sk_sndtimeo timeout value is specified in jiffes and should be >>>> converted to miliseconds when used as input to __set_chan_timer. >>>> >>>> Signed-off-by: Andrzej Kaczmarek<andrzej.kaczmarek@tieto.com> >>>> --- >>>> net/bluetooth/l2cap_core.c | 8 ++++---- >>>> 1 files changed, 4 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >>>> index f850684..3b0f807 100644 >>>> --- a/net/bluetooth/l2cap_core.c >>>> +++ b/net/bluetooth/l2cap_core.c >>>> @@ -446,7 +446,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason) >>>> if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED&& >>>> conn->hcon->type == ACL_LINK) { >>>> __clear_chan_timer(chan); >>>> - __set_chan_timer(chan, sk->sk_sndtimeo); >>>> + __set_chan_timer(chan, jiffies_to_msecs(sk->sk_sndtimeo)); >>> >>> Then __set_chan_timer do reverse conversion: >>> mod_timer(timer, jiffies + msecs_to_jiffies(timeout)) >>> >>> Look ugly :-( >> >> Well, I agree. But problem here is that __set_chan_timer has >> miliseconds as input and sk_sndtimeo has to be specified in jiffies >> thus ugly double conversion. > > Maybe we can just use jiffies in __set_chan_timer ? This will actually move conversion one step further to underlying l2cap_set_timer :-) And moreover we'll need to change other __set_chan_timer calls to use jiffies again and put msecs_to_jiffies "everywhere". Of course we can make also l2cap_set_timer to accept jiffies and set all timers (except sk_sndtimeo) with msecs_to_jiffies, but I think this will be even more ugly :-( BR, Andrzej ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value 2011-11-10 9:37 ` [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value Andrzej Kaczmarek 2011-11-10 9:57 ` Andrei Emeltchenko @ 2011-12-18 23:31 ` Gustavo Padovan 1 sibling, 0 replies; 8+ messages in thread From: Gustavo Padovan @ 2011-12-18 23:31 UTC (permalink / raw) To: Andrzej Kaczmarek Cc: linux-bluetooth, kanak.gupta, ulrik.lauren, henrik.possung Hi Andrzej. * Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com> [2011-11-10 10:37:25 +0100]: > sk_sndtimeo timeout value is specified in jiffes and should be > converted to miliseconds when used as input to __set_chan_timer. > > Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com> > --- > net/bluetooth/l2cap_core.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) I think those one doesn't make sense anymore after the move to workqueues. Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-12-18 23:31 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-10 9:37 [PATCH 0/2] Fix usage of sk_sndtimeo value in l2cap Andrzej Kaczmarek 2011-11-10 9:37 ` [PATCH 1/2] Buetooth: Revert "Fixed wrong L2CAP Sock timer value" Andrzej Kaczmarek 2011-11-10 9:37 ` [PATCH 2/2] Bluetooth: Fix usage of sk_sndtimeo value Andrzej Kaczmarek 2011-11-10 9:57 ` Andrei Emeltchenko 2011-11-10 10:13 ` Andrzej Kaczmarek 2011-11-10 10:35 ` Andrei Emeltchenko 2011-11-10 12:09 ` Andrzej Kaczmarek 2011-12-18 23:31 ` Gustavo Padovan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).