2008-09-29 18:17:52

by Rabin Vincent

[permalink] [raw]
Subject: Re: BUG kmalloc-16: Object already free

On Sun, Sep 28, 2008 at 03:54:23PM -0700, Justin Mattock wrote:
> After frying my system, I'm finally up and
> running. Not sure if this was due to a git-pull
> (only be a few days since the last pull), or what:
> when waking from suspend I see this
> (I know it says tainted in it, so this will be the only noise you'll
> here from me on this);
>
> [ 274.327003] =============================================================================
> [ 274.327528] BUG kmalloc-16: Object already free
> [ 274.327877] -----------------------------------------------------------------------------
> [ 274.327879]
> [ 274.327890] INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=0
> cpu=1 pid=3763
> [ 274.327899] INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=0
> cpu=1 pid=3763
> [ 274.327905] INFO: Slab 0xc139a100 objects=64 used=62 fp=0xdcd08100
> flags=0x400000c3

There's a commit in the latest git which looks like it will solve the
btusb suspend/resume issues: 5fbcd260.. ("[Bluetooth] Fix USB disconnect
handling of btusb driver").

Marcel / linux-bluetooth, I think this double free is a separate issue
with the error handling, and the following patch should fix it.

---
From: Rabin Vincent <[email protected]>
Subject: [PATCH] btusb, bpa10x: fix double frees on error paths

Justin Mattock reported this double free in btusb:

BUG kmalloc-16: Object already free
-----------------------------------------------------------------------------

INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763

This occurs because the urb's transfer buffer is being freed separately
in the error path even though the URB_FREE_BUFFER transfer_flag is set
on the urb.

There are similar cases elsewhere in btusb and in bpa10x. Fix all of
them by removing the additional kfree()'s.

Reported-by: Justin Mattock <[email protected]>
Signed-off-by: Rabin Vincent <[email protected]>
---
drivers/bluetooth/bpa10x.c | 2 --
drivers/bluetooth/btusb.c | 3 ---
2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index 1e55a65..32f3a8e 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -256,7 +256,6 @@ static inline int bpa10x_submit_intr_urb(struct hci_dev *hdev)
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
usb_unanchor_urb(urb);
- kfree(buf);
}

usb_free_urb(urb);
@@ -298,7 +297,6 @@ static inline int bpa10x_submit_bulk_urb(struct hci_dev *hdev)
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
usb_unanchor_urb(urb);
- kfree(buf);
}

usb_free_urb(urb);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 29ae998..262e9be 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -271,7 +271,6 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev)
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
usb_unanchor_urb(urb);
- kfree(buf);
}

usb_free_urb(urb);
@@ -354,7 +353,6 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev)
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
usb_unanchor_urb(urb);
- kfree(buf);
}

usb_free_urb(urb);
@@ -475,7 +473,6 @@ static int btusb_submit_isoc_urb(struct hci_dev *hdev)
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
usb_unanchor_urb(urb);
- kfree(buf);
}

usb_free_urb(urb);
--
1.5.6.5


2008-09-30 18:24:58

by Justin P. Mattock

[permalink] [raw]
Subject: Re: BUG kmalloc-16: Object already free

On Mon, Sep 29, 2008 at 10:21 PM, Justin Mattock
<[email protected]> wrote:
> On Mon, Sep 29, 2008 at 4:47 PM, Marcel Holtmann <[email protected]> wrote:
>> Hi Rabin,
>>
>>> > After frying my system, I'm finally up and
>>> > running. Not sure if this was due to a git-pull
>>> > (only be a few days since the last pull), or what:
>>> > when waking from suspend I see this
>>> > (I know it says tainted in it, so this will be the only noise you'll
>>> > here from me on this);
>>> >
>>> > [ 274.327003] =============================================================================
>>> > [ 274.327528] BUG kmalloc-16: Object already free
>>> > [ 274.327877] -----------------------------------------------------------------------------
>>> > [ 274.327879]
>>> > [ 274.327890] INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=0
>>> > cpu=1 pid=3763
>>> > [ 274.327899] INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=0
>>> > cpu=1 pid=3763
>>> > [ 274.327905] INFO: Slab 0xc139a100 objects=64 used=62 fp=0xdcd08100
>>> > flags=0x400000c3
>>>
>>> There's a commit in the latest git which looks like it will solve the
>>> btusb suspend/resume issues: 5fbcd260.. ("[Bluetooth] Fix USB disconnect
>>> handling of btusb driver").
>>>
>>> Marcel / linux-bluetooth, I think this double free is a separate issue
>>> with the error handling, and the following patch should fix it.
>>>
>>> ---
>>> From: Rabin Vincent <[email protected]>
>>> Subject: [PATCH] btusb, bpa10x: fix double frees on error paths
>>>
>>> Justin Mattock reported this double free in btusb:
>>>
>>> BUG kmalloc-16: Object already free
>>> -----------------------------------------------------------------------------
>>>
>>> INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
>>> INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
>>>
>>> This occurs because the urb's transfer buffer is being freed separately
>>> in the error path even though the URB_FREE_BUFFER transfer_flag is set
>>> on the urb.
>>>
>>> There are similar cases elsewhere in btusb and in bpa10x. Fix all of
>>> them by removing the additional kfree()'s.
>>
>> I haven't verified it yet, but it looks like a good catch. Let me double
>> check this on my test machine. Weird that we never noticed this before
>> since I have been using the btusb driver for a very long time now.
>>
>> Regards
>>
>> Marcel
>>
>>
>>
>
> This was the first time I've seen this,
> I can apply the patch myself, but first
> I need to figure why dbus can be such a bitch : )
> Need to figure out how to write dbus rules(if this is the case)
> keep getting the permissions denied crap.
>
> --
> Justin P. Mattock
>

O.k. after messing around with /etc/dbus
I've applied the patch that was supplied.
Looks good!! attached is a before the patch was
applied and after the patch was applied.

--
Justin P. Mattock


Attachments:
(No filename) (2.82 kB)
after (55.74 kB)
before (62.20 kB)
Download all attachments

2008-09-30 05:21:28

by Justin P. Mattock

[permalink] [raw]
Subject: Re: BUG kmalloc-16: Object already free

On Mon, Sep 29, 2008 at 4:47 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Rabin,
>
>> > After frying my system, I'm finally up and
>> > running. Not sure if this was due to a git-pull
>> > (only be a few days since the last pull), or what:
>> > when waking from suspend I see this
>> > (I know it says tainted in it, so this will be the only noise you'll
>> > here from me on this);
>> >
>> > [ 274.327003] =============================================================================
>> > [ 274.327528] BUG kmalloc-16: Object already free
>> > [ 274.327877] -----------------------------------------------------------------------------
>> > [ 274.327879]
>> > [ 274.327890] INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=0
>> > cpu=1 pid=3763
>> > [ 274.327899] INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=0
>> > cpu=1 pid=3763
>> > [ 274.327905] INFO: Slab 0xc139a100 objects=64 used=62 fp=0xdcd08100
>> > flags=0x400000c3
>>
>> There's a commit in the latest git which looks like it will solve the
>> btusb suspend/resume issues: 5fbcd260.. ("[Bluetooth] Fix USB disconnect
>> handling of btusb driver").
>>
>> Marcel / linux-bluetooth, I think this double free is a separate issue
>> with the error handling, and the following patch should fix it.
>>
>> ---
>> From: Rabin Vincent <[email protected]>
>> Subject: [PATCH] btusb, bpa10x: fix double frees on error paths
>>
>> Justin Mattock reported this double free in btusb:
>>
>> BUG kmalloc-16: Object already free
>> -----------------------------------------------------------------------------
>>
>> INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
>> INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
>>
>> This occurs because the urb's transfer buffer is being freed separately
>> in the error path even though the URB_FREE_BUFFER transfer_flag is set
>> on the urb.
>>
>> There are similar cases elsewhere in btusb and in bpa10x. Fix all of
>> them by removing the additional kfree()'s.
>
> I haven't verified it yet, but it looks like a good catch. Let me double
> check this on my test machine. Weird that we never noticed this before
> since I have been using the btusb driver for a very long time now.
>
> Regards
>
> Marcel
>
>
>

This was the first time I've seen this,
I can apply the patch myself, but first
I need to figure why dbus can be such a bitch : )
Need to figure out how to write dbus rules(if this is the case)
keep getting the permissions denied crap.

--
Justin P. Mattock

2008-09-29 23:47:39

by Marcel Holtmann

[permalink] [raw]
Subject: Re: BUG kmalloc-16: Object already free

Hi Rabin,

> > After frying my system, I'm finally up and
> > running. Not sure if this was due to a git-pull
> > (only be a few days since the last pull), or what:
> > when waking from suspend I see this
> > (I know it says tainted in it, so this will be the only noise you'll
> > here from me on this);
> >
> > [ 274.327003] =============================================================================
> > [ 274.327528] BUG kmalloc-16: Object already free
> > [ 274.327877] -----------------------------------------------------------------------------
> > [ 274.327879]
> > [ 274.327890] INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=0
> > cpu=1 pid=3763
> > [ 274.327899] INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=0
> > cpu=1 pid=3763
> > [ 274.327905] INFO: Slab 0xc139a100 objects=64 used=62 fp=0xdcd08100
> > flags=0x400000c3
>
> There's a commit in the latest git which looks like it will solve the
> btusb suspend/resume issues: 5fbcd260.. ("[Bluetooth] Fix USB disconnect
> handling of btusb driver").
>
> Marcel / linux-bluetooth, I think this double free is a separate issue
> with the error handling, and the following patch should fix it.
>
> ---
> From: Rabin Vincent <[email protected]>
> Subject: [PATCH] btusb, bpa10x: fix double frees on error paths
>
> Justin Mattock reported this double free in btusb:
>
> BUG kmalloc-16: Object already free
> -----------------------------------------------------------------------------
>
> INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
> INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
>
> This occurs because the urb's transfer buffer is being freed separately
> in the error path even though the URB_FREE_BUFFER transfer_flag is set
> on the urb.
>
> There are similar cases elsewhere in btusb and in bpa10x. Fix all of
> them by removing the additional kfree()'s.

I haven't verified it yet, but it looks like a good catch. Let me double
check this on my test machine. Weird that we never noticed this before
since I have been using the btusb driver for a very long time now.

Regards

Marcel



2008-09-29 19:22:05

by Justin P. Mattock

[permalink] [raw]
Subject: Re: BUG kmalloc-16: Object already free

On Mon, Sep 29, 2008 at 11:17 AM, Rabin Vincent <[email protected]> wrote:
> On Sun, Sep 28, 2008 at 03:54:23PM -0700, Justin Mattock wrote:
>> After frying my system, I'm finally up and
>> running. Not sure if this was due to a git-pull
>> (only be a few days since the last pull), or what:
>> when waking from suspend I see this
>> (I know it says tainted in it, so this will be the only noise you'll
>> here from me on this);
>>
>> [ 274.327003] =============================================================================
>> [ 274.327528] BUG kmalloc-16: Object already free
>> [ 274.327877] -----------------------------------------------------------------------------
>> [ 274.327879]
>> [ 274.327890] INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=0
>> cpu=1 pid=3763
>> [ 274.327899] INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=0
>> cpu=1 pid=3763
>> [ 274.327905] INFO: Slab 0xc139a100 objects=64 used=62 fp=0xdcd08100
>> flags=0x400000c3
>
> There's a commit in the latest git which looks like it will solve the
> btusb suspend/resume issues: 5fbcd260.. ("[Bluetooth] Fix USB disconnect
> handling of btusb driver").
>
> Marcel / linux-bluetooth, I think this double free is a separate issue
> with the error handling, and the following patch should fix it.
>
> ---
> From: Rabin Vincent <[email protected]>
> Subject: [PATCH] btusb, bpa10x: fix double frees on error paths
>
> Justin Mattock reported this double free in btusb:
>
> BUG kmalloc-16: Object already free
> -----------------------------------------------------------------------------
>
> INFO: Allocated in btusb_open+0x82/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
> INFO: Freed in btusb_open+0x13d/0x16f [btusb] age=3D0 cpu=3D1 pid=3D3763
>
> This occurs because the urb's transfer buffer is being freed separately
> in the error path even though the URB_FREE_BUFFER transfer_flag is set
> on the urb.
>
> There are similar cases elsewhere in btusb and in bpa10x. Fix all of
> them by removing the additional kfree()'s.
>
> Reported-by: Justin Mattock <[email protected]>
> Signed-off-by: Rabin Vincent <[email protected]>
> ---
> drivers/bluetooth/bpa10x.c | 2 --
> drivers/bluetooth/btusb.c | 3 ---
> 2 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
> index 1e55a65..32f3a8e 100644
> --- a/drivers/bluetooth/bpa10x.c
> +++ b/drivers/bluetooth/bpa10x.c
> @@ -256,7 +256,6 @@ static inline int bpa10x_submit_intr_urb(struct hci_dev *hdev)
> BT_ERR("%s urb %p submission failed (%d)",
> hdev->name, urb, -err);
> usb_unanchor_urb(urb);
> - kfree(buf);
> }
>
> usb_free_urb(urb);
> @@ -298,7 +297,6 @@ static inline int bpa10x_submit_bulk_urb(struct hci_dev *hdev)
> BT_ERR("%s urb %p submission failed (%d)",
> hdev->name, urb, -err);
> usb_unanchor_urb(urb);
> - kfree(buf);
> }
>
> usb_free_urb(urb);
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 29ae998..262e9be 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -271,7 +271,6 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev)
> BT_ERR("%s urb %p submission failed (%d)",
> hdev->name, urb, -err);
> usb_unanchor_urb(urb);
> - kfree(buf);
> }
>
> usb_free_urb(urb);
> @@ -354,7 +353,6 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev)
> BT_ERR("%s urb %p submission failed (%d)",
> hdev->name, urb, -err);
> usb_unanchor_urb(urb);
> - kfree(buf);
> }
>
> usb_free_urb(urb);
> @@ -475,7 +473,6 @@ static int btusb_submit_isoc_urb(struct hci_dev *hdev)
> BT_ERR("%s urb %p submission failed (%d)",
> hdev->name, urb, -err);
> usb_unanchor_urb(urb);
> - kfree(buf);
> }
>
> usb_free_urb(urb);
> --
> 1.5.6.5
>
>

Cool, depending on the status of
this patch. either I'll apply this one, or just wait
until it gets commited,and then just do a git-pull.

--
Justin P. Mattock