Re: [PATCH] Bluetooth: Fix unaligned access to l2cap conf data

Re: [PATCH] Bluetooth: Fix unaligned access to l2cap conf data

[real mz]

ping

On Mon, Aug 30, 2010 at 2:18 PM, steven.miao <realmz6@gmail.com> wrote:
> From: realmz <realmz6@gmail.com>
>
> In function l2cap_get_conf_opt() and l2cap_add_conf_opt() the address of
> opt->val sometimes is not at the edge of 2-bytes/4-bytes, so 2-bytes/4 bytes
> access will cause data misalignment exeception. Use get_unaligned_le16/32
> and put_unaligned_le16/32 function to avoid data misalignment execption.
> ---
>  net/bluetooth/l2cap.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index fadf26b..a07dad8 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -2406,11 +2406,11 @@ static inline int l2cap_get_conf_opt(void **ptr, int *type, int *olen, unsigned
>                break;
>
>        case 2:
> -               *val = __le16_to_cpu(*((__le16 *) opt->val));
> +               *val = get_unaligned_le16(opt->val);
>                break;
>
>        case 4:
> -               *val = __le32_to_cpu(*((__le32 *) opt->val));
> +               *val = get_unaligned_le32(opt->val);
>                break;
>
>        default:
> @@ -2437,11 +2437,11 @@ static void l2cap_add_conf_opt(void **ptr, u8 type, u8 len, unsigned long val)
>                break;
>
>        case 2:
> -               *((__le16 *) opt->val) = cpu_to_le16(val);
> +               put_unaligned_le16(cpu_to_le16(val), opt->val);
>                break;
>
>        case 4:
> -               *((__le32 *) opt->val) = cpu_to_le32(val);
> +               put_unaligned_le32(cpu_to_le32(val), opt->val);
>                break;
>
>        default:
> --
> 1.5.6.5
>
>

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Mike Frysinger]

On Mon, Oct 18, 2010 at 15:12, Gustavo F. Padovan wrote:
> * Mike Frysinger <vapier@gentoo.org> [2010-10-18 15:10:36 -0400]:
>> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
>> > * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]:
>> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> >> > From: steven miao <realmz6@gmail.com>
>> >> >
>> >> >        case 2:
>> >> > -               *((__le16 *) opt->val) = cpu_to_le16(val);
>> >> > +               put_unaligned_le16(cpu_to_le16(val), opt->val);
>> >> >                break;
>> >>
>> >> I think you wanted:
>> >> put_unaligned_le16(val, opt->val);
>> >
>> > I fixed that in the tree. Thanks for the report.
>>
>> i guess you fixed the 32bit one too ?
>>   put_unaligned_le32(cpu_to_le32(val), opt->val);
>
> Yes, I did.

cool.  thanks guys !
-mike

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Mike Frysinger]

On Mon, Oct 18, 2010 at 15:12, Gustavo F. Padovan wrote:
> * Mike Frysinger <vapier@gentoo.org> [2010-10-18 15:10:36 -0400]:
>> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
>> > * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -07=
00]:
>> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> w=
rote:
>> >> > From: steven miao <realmz6@gmail.com>
>> >> >
>> >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0case 2:
>> >> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *((__le16 *) opt=
->val) =3D cpu_to_le16(val);
>> >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 put_unaligned_le=
16(cpu_to_le16(val), opt->val);
>> >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>> >>
>> >> I think you wanted:
>> >> put_unaligned_le16(val, opt->val);
>> >
>> > I fixed that in the tree. Thanks for the report.
>>
>> i guess you fixed the 32bit one too ?
>> =C2=A0 put_unaligned_le32(cpu_to_le32(val), opt->val);
>
> Yes, I did.

cool.  thanks guys !
-mike

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Gustavo F. Padovan]

* Mike Frysinger <vapier@gentoo.org> [2010-10-18 15:10:36 -0400]:

> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> > * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]:
> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> >> > From: steven miao <realmz6@gmail.com>
> >> >
> >> >        case 2:
> >> > -               *((__le16 *) opt->val) = cpu_to_le16(val);
> >> > +               put_unaligned_le16(cpu_to_le16(val), opt->val);
> >> >                break;
> >>
> >> I think you wanted:
> >> put_unaligned_le16(val, opt->val);
> >
> > I fixed that in the tree. Thanks for the report.
> 
> i guess you fixed the 32bit one too ?
>   put_unaligned_le32(cpu_to_le32(val), opt->val);

Yes, I did.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Gustavo F. Padovan]

* Mike Frysinger <vapier@gentoo.org> [2010-10-18 15:10:36 -0400]:

> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> > * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -070=
0]:
> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wr=
ote:
> >> > From: steven miao <realmz6@gmail.com>
> >> >
> >> > =A0 =A0 =A0 =A0case 2:
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 *((__le16 *) opt->val) =3D cpu_to_le16=
(val);
> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 put_unaligned_le16(cpu_to_le16(val), o=
pt->val);
> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> >>
> >> I think you wanted:
> >> put_unaligned_le16(val, opt->val);
> >
> > I fixed that in the tree. Thanks for the report.
>=20
> i guess you fixed the 32bit one too ?
>   put_unaligned_le32(cpu_to_le32(val), opt->val);

Yes, I did.

--=20
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Mike Frysinger]

On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]:
>> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> > From: steven miao <realmz6@gmail.com>
>> >
>> >        case 2:
>> > -               *((__le16 *) opt->val) = cpu_to_le16(val);
>> > +               put_unaligned_le16(cpu_to_le16(val), opt->val);
>> >                break;
>>
>> I think you wanted:
>> put_unaligned_le16(val, opt->val);
>
> I fixed that in the tree. Thanks for the report.

i guess you fixed the 32bit one too ?
  put_unaligned_le32(cpu_to_le32(val), opt->val);
-mike

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Mike Frysinger]

On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]=
:
>> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrot=
e:
>> > From: steven miao <realmz6@gmail.com>
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0case 2:
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *((__le16 *) opt->v=
al) =3D cpu_to_le16(val);
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 put_unaligned_le16(=
cpu_to_le16(val), opt->val);
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>>
>> I think you wanted:
>> put_unaligned_le16(val, opt->val);
>
> I fixed that in the tree. Thanks for the report.

i guess you fixed the 32bit one too ?
  put_unaligned_le32(cpu_to_le32(val), opt->val);
-mike

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Harvey Harrison]

On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> From: steven miao <realmz6@gmail.com>
>
>
>        case 2:
> -               *((__le16 *) opt->val) = cpu_to_le16(val);
> +               put_unaligned_le16(cpu_to_le16(val), opt->val);
>                break;
>

I think you wanted:
put_unaligned_le16(val, opt->val);

Cheers,

Harvey

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Harvey Harrison]

On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> From: steven miao <realmz6@gmail.com>
>
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0case 2:
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *((__le16 *) opt->val)=
 =3D cpu_to_le16(val);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 put_unaligned_le16(cpu=
_to_le16(val), opt->val);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>

I think you wanted:
put_unaligned_le16(val, opt->val);

Cheers,

Harvey

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Gustavo F. Padovan]

Hi Harvey,

* Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]:

> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > From: steven miao <realmz6@gmail.com>
> >
> >
> >        case 2:
> > -               *((__le16 *) opt->val) = cpu_to_le16(val);
> > +               put_unaligned_le16(cpu_to_le16(val), opt->val);
> >                break;
> >
> 
> I think you wanted:
> put_unaligned_le16(val, opt->val);

I fixed that in the tree. Thanks for the report. 

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Gustavo F. Padovan]

Hi Harvey,

* Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]:

> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > From: steven miao <realmz6@gmail.com>
> >
> >
> > =A0 =A0 =A0 =A0case 2:
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 *((__le16 *) opt->val) =3D cpu_to_le16(va=
l);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 put_unaligned_le16(cpu_to_le16(val), opt-=
>val);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> >
>=20
> I think you wanted:
> put_unaligned_le16(val, opt->val);

I fixed that in the tree. Thanks for the report.=20

--=20
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Gustavo F. Padovan]

Hi Mike,

* Marcel Holtmann <marcel@holtmann.org> [2010-10-18 13:51:46 +0200]:

> Hi Mike,
> 
> > In function l2cap_get_conf_opt() and l2cap_add_conf_opt() the address of
> > opt->val sometimes is not at the edge of 2-bytes/4-bytes, so 2-bytes/4 bytes
> > access will cause data misalignment exeception.  Use get_unaligned_le16/32
> > and put_unaligned_le16/32 function to avoid data misalignment execption.
> > 
> > Signed-off-by: steven miao <realmz6@gmail.com>
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > was posted a month ago with no feedback ...
> 
> must have slipped through. However I don't remember it being on
> linux-bluetooth at all. Maybe it was on the other mailing lists :(

Actually I remember it, but it got lost by some way. :(

> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>

Applied, thanks.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data

[Marcel Holtmann]

Hi Mike,

> In function l2cap_get_conf_opt() and l2cap_add_conf_opt() the address of
> opt->val sometimes is not at the edge of 2-bytes/4-bytes, so 2-bytes/4 bytes
> access will cause data misalignment exeception.  Use get_unaligned_le16/32
> and put_unaligned_le16/32 function to avoid data misalignment execption.
> 
> Signed-off-by: steven miao <realmz6@gmail.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> was posted a month ago with no feedback ...

must have slipped through. However I don't remember it being on
linux-bluetooth at all. Maybe it was on the other mailing lists :(

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

[PATCH] bluetooth: fix unaligned access to l2cap conf data

[Mike Frysinger]

From: steven miao <realmz6@gmail.com>

In function l2cap_get_conf_opt() and l2cap_add_conf_opt() the address of
opt->val sometimes is not at the edge of 2-bytes/4-bytes, so 2-bytes/4 bytes
access will cause data misalignment exeception.  Use get_unaligned_le16/32
and put_unaligned_le16/32 function to avoid data misalignment execption.

Signed-off-by: steven miao <realmz6@gmail.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
was posted a month ago with no feedback ...

 net/bluetooth/l2cap.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 0b54b7d..65bcdc1 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2401,11 +2401,11 @@ static inline int l2cap_get_conf_opt(void **ptr, int *type, int *olen, unsigned
 		break;
 
 	case 2:
-		*val = __le16_to_cpu(*((__le16 *) opt->val));
+		*val = get_unaligned_le16(opt->val);
 		break;
 
 	case 4:
-		*val = __le32_to_cpu(*((__le32 *) opt->val));
+		*val = get_unaligned_le32(opt->val);
 		break;
 
 	default:
@@ -2432,11 +2432,11 @@ static void l2cap_add_conf_opt(void **ptr, u8 type, u8 len, unsigned long val)
 		break;
 
 	case 2:
-		*((__le16 *) opt->val) = cpu_to_le16(val);
+		put_unaligned_le16(cpu_to_le16(val), opt->val);
 		break;
 
 	case 4:
-		*((__le32 *) opt->val) = cpu_to_le32(val);
+		put_unaligned_le32(cpu_to_le32(val), opt->val);
 		break;
 
 	default:
-- 
1.7.3.1