Netdev Archive mirror
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: Silence static checker warning.
@ 2012-02-29 17:06 santosh nayak
       [not found] ` <1330535217-26785-1-git-send-email-santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: santosh nayak @ 2012-02-29 17:06 UTC (permalink / raw
  To: marcel
  Cc: padovan, davem, linux-bluetooth, netdev, linux-kernel,
	kernel-janitors, Santosh Nayak

From: Santosh Nayak <santoshprasadnayak@gmail.com>

Silencing Static checker warning.
1. Endian warning
2. variable dereferenced before check 'sk' .

Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
---
 net/bluetooth/l2cap_sock.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 401d942..d206321 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -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));
 	else
 		err = 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 al
 	if (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)
 		goto done;
 
@@ -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)
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
       [not found] ` <1330535217-26785-1-git-send-email-santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2012-02-29 17:39   ` Marcel Holtmann
  2012-02-29 17:46     ` Marcel Holtmann
  0 siblings, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2012-02-29 17:39 UTC (permalink / raw
  To: santosh nayak
  Cc: padovan-Y3ZbgMPKUGA34EUeqzHoZw, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

Hi Santosh,

> Silencing Static checker warning.
> 1. Endian warning
> 2. variable dereferenced before check 'sk' .
> 
> Signed-off-by: Santosh Nayak <santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  net/bluetooth/l2cap_sock.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 401d942..d206321 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -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));
>  	else
>  		err = 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 al
>  	if (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)
>  		goto done;

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.
 
> @@ -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)

This one is fine.

Regards

Marcel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
  2012-02-29 17:39   ` Marcel Holtmann
@ 2012-02-29 17:46     ` Marcel Holtmann
  2012-03-02  9:15       ` David Laight
  0 siblings, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2012-02-29 17:46 UTC (permalink / raw
  To: santosh nayak
  Cc: padovan, davem, linux-bluetooth, netdev, linux-kernel,
	kernel-janitors

Hi Santosh,

> > Silencing Static checker warning.
> > 1. Endian warning
> > 2. variable dereferenced before check 'sk' .
> > 
> > Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
> > ---
> >  net/bluetooth/l2cap_sock.c |    9 ++++++---
> >  1 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 401d942..d206321 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -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));
> >  	else
> >  		err = 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 al
> >  	if (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)
> >  		goto done;
> 
> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
  2012-02-29 17:06 [PATCH] Bluetooth: Silence static checker warning santosh nayak
       [not found] ` <1330535217-26785-1-git-send-email-santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2012-02-29 19:16 ` Dan Carpenter
  2012-02-29 20:45 ` Andrei Emeltchenko
  2 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2012-02-29 19:16 UTC (permalink / raw
  To: santosh nayak
  Cc: marcel, padovan, davem, linux-bluetooth, netdev, linux-kernel,
	kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

On Wed, Feb 29, 2012 at 10:36:57PM +0530, santosh nayak wrote:
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
> 
> Silencing Static checker warning.
> 1. Endian warning

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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
  2012-02-29 17:06 [PATCH] Bluetooth: Silence static checker warning santosh nayak
       [not found] ` <1330535217-26785-1-git-send-email-santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2012-02-29 19:16 ` Dan Carpenter
@ 2012-02-29 20:45 ` Andrei Emeltchenko
       [not found]   ` <CAMXE1-oXtheVHDuU3w52mXqmuY8AK4kk+tuvJpVRDq2=+qEnsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2 siblings, 1 reply; 11+ messages in thread
From: Andrei Emeltchenko @ 2012-02-29 20:45 UTC (permalink / raw
  To: santosh nayak
  Cc: marcel, padovan, davem, linux-bluetooth, netdev, linux-kernel,
	kernel-janitors

Hi Santosh,

On Wed, Feb 29, 2012 at 7:06 PM, santosh nayak
<santoshprasadnayak@gmail.com> wrote:
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
>
> Silencing Static checker warning.
> 1. Endian warning
> 2. variable dereferenced before check 'sk' .
>
> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
> ---
>  net/bluetooth/l2cap_sock.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 401d942..d206321 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -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));
>        else
>                err = 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 al
>        if (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)
>                goto done;
>
> @@ -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;
> +

Didn't I fix this bug already?

http://permalink.gmane.org/gmane.linux.bluez.kernel/21537

Regards,
Andrei

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
       [not found]   ` <CAMXE1-oXtheVHDuU3w52mXqmuY8AK4kk+tuvJpVRDq2=+qEnsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2012-03-01  5:37     ` santosh prasad nayak
  2012-03-01  6:25       ` Dan Carpenter
  2012-03-01  7:43       ` Andrei Emeltchenko
  0 siblings, 2 replies; 11+ messages in thread
From: santosh prasad nayak @ 2012-03-01  5:37 UTC (permalink / raw
  To: Andrei Emeltchenko
  Cc: marcel-kz+m5ild9QBg9hUCZPvPmw, padovan-Y3ZbgMPKUGA34EUeqzHoZw,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

@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
<andrei.emeltchenko.news-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Santosh,
>
> On Wed, Feb 29, 2012 at 7:06 PM, santosh nayak
> <santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Santosh Nayak <santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Silencing Static checker warning.
>> 1. Endian warning
>> 2. variable dereferenced before check 'sk' .
>>
>> Signed-off-by: Santosh Nayak <santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  net/bluetooth/l2cap_sock.c |    9 ++++++---
>>  1 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>> index 401d942..d206321 100644
>> --- a/net/bluetooth/l2cap_sock.c
>> +++ b/net/bluetooth/l2cap_sock.c
>> @@ -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));
>>        else
>>                err = 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 al
>>        if (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)
>>                goto done;
>>
>> @@ -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;
>> +
>
> Didn't I fix this bug already?
>
> http://permalink.gmane.org/gmane.linux.bluez.kernel/21537
>
> Regards,
> Andrei

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
  2012-03-01  5:37     ` santosh prasad nayak
@ 2012-03-01  6:25       ` Dan Carpenter
  2012-03-01  7:43       ` Andrei Emeltchenko
  1 sibling, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2012-03-01  6:25 UTC (permalink / raw
  To: santosh prasad nayak
  Cc: Andrei Emeltchenko, marcel, padovan, davem, linux-bluetooth,
	netdev, linux-kernel, kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

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.
> 

Nope.  Andrei's patch is already in linux-next so you'll have to
redo this.

> @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 ?
> 

It's updated every day.  http://linux.f-seidel.de/linux-next/pmwiki/

regards,
dan carpenter


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
  2012-03-01  5:37     ` santosh prasad nayak
  2012-03-01  6:25       ` Dan Carpenter
@ 2012-03-01  7:43       ` Andrei Emeltchenko
  1 sibling, 0 replies; 11+ messages in thread
From: Andrei Emeltchenko @ 2012-03-01  7:43 UTC (permalink / raw
  To: santosh prasad nayak
  Cc: marcel, padovan, davem, linux-bluetooth, netdev, linux-kernel,
	kernel-janitors

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 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] Bluetooth: Silence static checker warning.
  2012-02-29 17:46     ` Marcel Holtmann
@ 2012-03-02  9:15       ` David Laight
  2012-03-02 11:04         ` Dan Carpenter
  0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2012-03-02  9:15 UTC (permalink / raw
  To: Marcel Holtmann, santosh nayak
  Cc: padovan, davem, linux-bluetooth, netdev, linux-kernel,
	kernel-janitors

 
> > > diff --git a/net/bluetooth/l2cap_sock.c
b/net/bluetooth/l2cap_sock.c
> > > index 401d942..d206321 100644
> > > --- a/net/bluetooth/l2cap_sock.c
> > > +++ b/net/bluetooth/l2cap_sock.c
> > > @@ -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));
> > >  	else
> > >  		err = 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 al
> > >  	if (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)
> > >  		goto done;
> > 
> > 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.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
  2012-03-02  9:15       ` David Laight
@ 2012-03-02 11:04         ` Dan Carpenter
  2012-03-02 17:57           ` Marcel Holtmann
  0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2012-03-02 11:04 UTC (permalink / raw
  To: David Laight
  Cc: Marcel Holtmann, santosh nayak, padovan, davem, linux-bluetooth,
	netdev, linux-kernel, kernel-janitors, Ville Tervo

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Bluetooth: Silence static checker warning.
  2012-03-02 11:04         ` Dan Carpenter
@ 2012-03-02 17:57           ` Marcel Holtmann
  0 siblings, 0 replies; 11+ messages in thread
From: Marcel Holtmann @ 2012-03-02 17:57 UTC (permalink / raw
  To: Dan Carpenter
  Cc: David Laight, santosh nayak, padovan, davem, linux-bluetooth,
	netdev, linux-kernel, kernel-janitors, Ville Tervo

Hi Dan,

> > 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-03-02 17:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-29 17:06 [PATCH] Bluetooth: Silence static checker warning santosh nayak
     [not found] ` <1330535217-26785-1-git-send-email-santoshprasadnayak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-02-29 17:39   ` Marcel Holtmann
2012-02-29 17:46     ` Marcel Holtmann
2012-03-02  9:15       ` David Laight
2012-03-02 11:04         ` Dan Carpenter
2012-03-02 17:57           ` Marcel Holtmann
2012-02-29 19:16 ` Dan Carpenter
2012-02-29 20:45 ` Andrei Emeltchenko
     [not found]   ` <CAMXE1-oXtheVHDuU3w52mXqmuY8AK4kk+tuvJpVRDq2=+qEnsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-03-01  5:37     ` santosh prasad nayak
2012-03-01  6:25       ` Dan Carpenter
2012-03-01  7:43       ` Andrei Emeltchenko

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 read-only IMAP folder(s) and NNTP newsgroup(s).