2010-10-16 22:29:47

by Mike Frysinger

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

From: steven miao <[email protected]>

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 <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
---
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

2010-10-18 19:39:11

by Mike Frysinger

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

On Mon, Oct 18, 2010 at 15:12, Gustavo F. Padovan wrote:
> * Mike Frysinger <[email protected]> [2010-10-18 15:10:36 -0400]:
>> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
>> > * Harvey Harrison <[email protected]> [2010-10-18 11:17:28 -07=
00]:
>> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <[email protected]> w=
rote:
>> >> > From: steven miao <[email protected]>
>> >> >
>> >> > =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

2010-10-18 19:12:10

by Gustavo Padovan

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

* Mike Frysinger <[email protected]> [2010-10-18 15:10:36 -0400]:

> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> > * Harvey Harrison <[email protected]> [2010-10-18 11:17:28 -070=
0]:
> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <[email protected]> wr=
ote:
> >> > From: steven miao <[email protected]>
> >> >
> >> > =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

2010-10-18 19:10:36

by Mike Frysinger

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

On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> * Harvey Harrison <[email protected]> [2010-10-18 11:17:28 -0700]=
:
>> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <[email protected]> wrot=
e:
>> > From: steven miao <[email protected]>
>> >
>> > =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

2010-10-18 16:32:23

by Gustavo Padovan

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

Hi Harvey,

* Harvey Harrison <[email protected]> [2010-10-18 11:17:28 -0700]:

> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <[email protected]> wrote:
> > From: steven miao <[email protected]>
> >
> >
> > =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

2010-10-18 18:17:28

by Harvey Harrison

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

On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <[email protected]> wrote:
> From: steven miao <[email protected]>
>
>
> =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

2010-10-18 15:59:16

by Gustavo Padovan

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

Hi Mike,

* Marcel Holtmann <[email protected]> [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 <[email protected]>
> > Signed-off-by: Mike Frysinger <[email protected]>
> > ---
> > 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 <[email protected]>

Applied, thanks.

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

2010-10-18 11:51:46

by Marcel Holtmann

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

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 <[email protected]>
> Signed-off-by: Mike Frysinger <[email protected]>
> ---
> 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 <[email protected]>

Regards

Marcel