From: Santosh Nayak <[email protected]>
Silencing Static checker warning.
1. Endian warning
2. variable dereferenced before check 'sk' .
Signed-off-by: Santosh Nayak <[email protected]>
---
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
Hi Santosh,
> Silencing Static checker warning.
> 1. Endian warning
> 2. variable dereferenced before check 'sk' .
>
> Signed-off-by: Santosh Nayak <[email protected]>
> ---
> 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
Hi Santosh,
> > Silencing Static checker warning.
> > 1. Endian warning
> > 2. variable dereferenced before check 'sk' .
> >
> > Signed-off-by: Santosh Nayak <[email protected]>
> > ---
> > 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 <[email protected]>
Regards
Marcel
On Wed, Feb 29, 2012 at 10:36:57PM +0530, santosh nayak wrote:
> From: Santosh Nayak <[email protected]>
>
> 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
Hi Santosh,
On Wed, Feb 29, 2012 at 7:06 PM, santosh nayak
<[email protected]> wrote:
> From: Santosh Nayak <[email protected]>
>
> Silencing Static checker warning.
> 1. Endian warning
> 2. variable dereferenced before check 'sk' .
>
> Signed-off-by: Santosh Nayak <[email protected]>
> ---
> ?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