@@ -82,7 +82,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)}if(la.l2_cid)-err=l2cap_add_scid(chan,la.l2_cid);+err=l2cap_add_scid(chan,__le16_to_cpu(la.l2_cid));elseerr=l2cap_add_psm(chan,&la.l2_bdaddr,la.l2_psm);
@@ -123,7 +123,8 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int alif(la.l2_cid&&la.l2_psm)return-EINVAL;-err=l2cap_chan_connect(chan,la.l2_psm,la.l2_cid,&la.l2_bdaddr);+err=l2cap_chan_connect(chan,la.l2_psm,__le16_to_cpu(la.l2_cid),+&la.l2_bdaddr);if(err)gotodone;
I am not sure about this one. Need to go back and read through the
source code. The value provided from userspace is already in the right
host endian. Could be that we mess up our internal classification. And
instead of adding __le16_to_cpu we should fix its classification.
quoted hunk
@@ -795,7 +796,7 @@ static void l2cap_sock_kill(struct sock *sk) static int l2cap_sock_shutdown(struct socket *sock, int how) { struct sock *sk = sock->sk;- struct l2cap_chan *chan = l2cap_pi(sk)->chan;+ struct l2cap_chan *chan; int err = 0; BT_DBG("sock %p, sk %p", sock, sk);
@@ -803,6 +804,8 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) if (!sk) return 0;+ chan = l2cap_pi(sk)->chan;+ lock_sock(sk); if (!sk->sk_shutdown) { if (chan->mode == L2CAP_MODE_ERTM)
@@ -82,7 +82,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)}if(la.l2_cid)-err=l2cap_add_scid(chan,la.l2_cid);+err=l2cap_add_scid(chan,__le16_to_cpu(la.l2_cid));elseerr=l2cap_add_psm(chan,&la.l2_bdaddr,la.l2_psm);
@@ -123,7 +123,8 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int alif(la.l2_cid&&la.l2_psm)return-EINVAL;-err=l2cap_chan_connect(chan,la.l2_psm,la.l2_cid,&la.l2_bdaddr);+err=l2cap_chan_connect(chan,la.l2_psm,__le16_to_cpu(la.l2_cid),+&la.l2_bdaddr);if(err)gotodone;
I am not sure about this one. Need to go back and read through the
source code. The value provided from userspace is already in the right
host endian. Could be that we mess up our internal classification. And
instead of adding __le16_to_cpu we should fix its classification.
I confused myself here, so the provided PSM and CID values coming from
userspace are little endian. Patch is correct.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
It's not an endian warning, it's an endian bug. This code won't
work on big endian systems. Don't mix bugfixes and other changes.
Probably at some point someone will be updating their kernel for
an embedded platform and they'll do a "git log --pretty=oneline" and
they'll notice your endian fix and decide it's super important to
them. Right now it's hard to find.
regards,
dan carpenter
@Dan,
In future patches I will take care of it. Is it ok ? If
required I can resend the patch with required changes on subject line.
@Andrei
In my local clone your changes are not visible.
What is the schedule of linux-next ?
Is it updated every week or bi-weekly or monthly ?
Regards
Santosh
On Thu, Mar 1, 2012 at 2:15 AM, Andrei Emeltchenko
[off-list ref] wrote:
Hi Santosh,
On Wed, Feb 29, 2012 at 7:06 PM, santosh nayak
[off-list ref] wrote:
From: Andrei Emeltchenko <hidden> Date: 2012-03-01 07:44:07
Hi Santosh,
On Thu, Mar 01, 2012 at 11:07:14AM +0530, santosh prasad nayak wrote:
@Dan,
In future patches I will take care of it. Is it ok ? If
required I can resend the patch with required changes on subject line.
@Andrei
In my local clone your changes are not visible.
What is the schedule of linux-next ?
Is it updated every week or bi-weekly or monthly ?
We use Johan's tree so far:
git://git.kernel.org/pub/scm/linux/kernel/git/jh/bluetooth-next.git
I think the tree we use shall be published on bluez.org website.
The change seems to be included to:
"pull request: bluetooth-next 2012-02-24"
http://www.spinics.net/lists/linux-wireless/msg85442.html
Best regards
Andrei Emeltchenko
I am not sure about this one. Need to go back and read through the
source code. The value provided from userspace is already in the
right
quoted
host endian. Could be that we mess up our internal classification.
And
quoted
instead of adding __le16_to_cpu we should fix its classification.
I confused myself here, so the provided PSM and CID values coming from
userspace are little endian. Patch is correct.
How long has this code been in tree?
It isn't obvious to me that this change won't break code on BE systems
where the application code is already fixing the endianness.
David
From: Dan Carpenter <hidden> Date: 2012-03-02 11:02:34
On Fri, Mar 02, 2012 at 09:15:37AM -0000, David Laight wrote:
How long has this code been in tree?
It isn't obvious to me that this change won't break code on BE systems
where the application code is already fixing the endianness.
It looks like we've had an endian bug since last February.
b62f328b8f20a "Bluetooth: Add server socket support for LE
connection"
+ l2cap_pi(sk)->scid = la.l2_cid;
->scid was cpu endian.
regards,
dan carpenter
How long has this code been in tree?
It isn't obvious to me that this change won't break code on BE systems
where the application code is already fixing the endianness.
It looks like we've had an endian bug since last February.
b62f328b8f20a "Bluetooth: Add server socket support for LE
connection"
+ l2cap_pi(sk)->scid = la.l2_cid;
->scid was cpu endian.
this is a bug. No questions asked.
However you can only exercise this code if you work with Bluetooth Low
Energy and that is not enabled by default since it is not fully finished
yet. CID is only used by Low Energy.
Bluetooth BR/EDR only uses PSM part of the socket address and that has
been endian safe.
Regards
Marcel