RE: [PATCH] Bluetooth: Fix for double free of st buffer.

Ping 

-----Original Message-----
From: Malovany, Ram
Sent: Wednesday, July 25, 2012 11:31 AM
To: 'Gustavo Padovan'
Cc: 'linux-bluetooth@vger.kernel.org'
Subject: RE: [PATCH] Bluetooth: Fix for double free of st buffer.

HI Gustavo,

quoted

-----Original Message-----
From: Malovany, Ram
Sent: Wednesday, July 25, 2012 11:01 AM
To: 'Gustavo Padovan'
Cc: linux-bluetooth@vger.kernel.org
Subject: RE: [PATCH] Bluetooth: Fix for double free of st buffer.

HI Gustavo,

quoted

-----Original Message-----
From: Gustavo Padovan [mailto:gustavo@padovan.org]
Sent: Wednesday, July 25, 2012 2:01 AM
To: Malovany, Ram
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: Fix for double free of st buffer.

Hi Ram,

* ramm@ti.com [off-list ref] [2012-07-17 10:49:30 +0300]:

quoted

From: Ram Malovany <redacted>

When the Shared Transport line discipline driver (st_core) get
data it pushes the skb received to the relevant protocol stacks ,
it then excepts that the relevant protocol stacks should handle
the buffer , and if it cannot then the stack should respond with an

error.

quoted

quoted

quoted

In our case the Bluetooth driver for shared transport (btwilink)
should always be able to handle the buffer , in case of an error
it will release it , thus we always should return 0.

Signed-off-by: Ram Malovany <redacted>
---
 drivers/bluetooth/btwilink.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btwilink.c

b/drivers/bluetooth/btwilink.c index 8869469..1f60d84 100644

--- a/drivers/bluetooth/btwilink.c
+++ b/drivers/bluetooth/btwilink.c

@@ -94,18 +94,22 @@ static void st_reg_completion_cb(void

*priv_data, char

data)

quoted

 }

 /* Called by Shared Transport layer when receive data is
- * available */
+ * available
+ * Return:
+ * 0 if buffer handled (affectivly allways even if error found)
+ * if return !=0 the buffer will be freed by the st */
 static long st_receive(void *priv_data, struct sk_buff *skb)  {
 	struct ti_st *lhst = priv_data;
 	int err;

 	if (!skb)
-		return -EFAULT;
+		return 0;

 	if (!lhst) {
 		kfree_skb(skb);

If you remove this call to kfree_skb() from here doesn't it fixes
your problem?

	Gustavo

I looked at the fix again , and I think we shouldn't change it since the main
problem that I am observing her is not the kfree_skb() at this point , The
problem is related to who is in charge of the buffer at this point , and in
our case the function hci_recv_frame() can use different transports and upon
an error it will free the buffer as well , in my fix I made sure that
st_receive() will always take care of the buffer.

quoted

You are correct , I was going to send a new version for this patch.

Thanks,
Ram