* [PATCH] Bluetooth: Fix for double free of st buffer.
@ 2012-07-17 7:49 ramm
2012-07-24 23:01 ` Gustavo Padovan
0 siblings, 1 reply; 5+ messages in thread
From: ramm @ 2012-07-17 7:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ram Malovany
From: Ram Malovany <ramm@ti.com>
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.
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 <ramm@ti.com>
---
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)
}
/* 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);
- return -EFAULT;
+ return 0;
}
skb->dev = (void *) lhst->hdev;
@@ -114,7 +118,7 @@ static long st_receive(void *priv_data, struct sk_buff *skb)
err = hci_recv_frame(skb);
if (err < 0) {
BT_ERR("Unable to push skb to HCI core(%d)", err);
- return err;
+ return 0;
}
lhst->hdev->stat.byte_rx += skb->len;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: Fix for double free of st buffer.
2012-07-17 7:49 [PATCH] Bluetooth: Fix for double free of st buffer ramm
@ 2012-07-24 23:01 ` Gustavo Padovan
2012-07-25 8:00 ` Malovany, Ram
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Gustavo Padovan @ 2012-07-24 23:01 UTC (permalink / raw)
To: ramm; +Cc: linux-bluetooth
Hi Ram,
* ramm@ti.com <ramm@ti.com> [2012-07-17 10:49:30 +0300]:
> From: Ram Malovany <ramm@ti.com>
>
> 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.
> 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 <ramm@ti.com>
> ---
> 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)
> }
>
> /* 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] Bluetooth: Fix for double free of st buffer.
2012-07-24 23:01 ` Gustavo Padovan
@ 2012-07-25 8:00 ` Malovany, Ram
2012-07-25 8:30 ` Malovany, Ram
2012-08-03 11:40 ` Malovany, Ram
2 siblings, 0 replies; 5+ messages in thread
From: Malovany, Ram @ 2012-07-25 8:00 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth
HI Gustavo,
> -----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 <ramm@ti.com> [2012-07-17 10:49:30 +0300]:
>
> > From: Ram Malovany <ramm@ti.com>
> >
> > 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.
> > 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 <ramm@ti.com>
> > ---
> > 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)
> > }
> >
> > /* 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
You are correct , I was going to send a new version for this patch.
Thanks,
Ram
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] Bluetooth: Fix for double free of st buffer.
2012-07-24 23:01 ` Gustavo Padovan
2012-07-25 8:00 ` Malovany, Ram
@ 2012-07-25 8:30 ` Malovany, Ram
2012-08-03 11:40 ` Malovany, Ram
2 siblings, 0 replies; 5+ messages in thread
From: Malovany, Ram @ 2012-07-25 8:30 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth
HI Gustavo,
> -----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,
>
> > -----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 <ramm@ti.com> [2012-07-17 10:49:30 +0300]:
> >
> > > From: Ram Malovany <ramm@ti.com>
> > >
> > > 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.
> > > 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 <ramm@ti.com>
> > > ---
> > > 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)
> > > }
> > >
> > > /* 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.
>
> You are correct , I was going to send a new version for this patch.
>
> Thanks,
> Ram
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] Bluetooth: Fix for double free of st buffer.
2012-07-24 23:01 ` Gustavo Padovan
2012-07-25 8:00 ` Malovany, Ram
2012-07-25 8:30 ` Malovany, Ram
@ 2012-08-03 11:40 ` Malovany, Ram
2 siblings, 0 replies; 5+ messages in thread
From: Malovany, Ram @ 2012-08-03 11:40 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth
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,
>
> > -----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,
> >
> > > -----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 <ramm@ti.com> [2012-07-17 10:49:30 +0300]:
> > >
> > > > From: Ram Malovany <ramm@ti.com>
> > > >
> > > > 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.
> > > > 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 <ramm@ti.com>
> > > > ---
> > > > 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)
> > > > }
> > > >
> > > > /* 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.
> >
> > You are correct , I was going to send a new version for this patch.
> >
> > Thanks,
> > Ram
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-08-03 11:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-17 7:49 [PATCH] Bluetooth: Fix for double free of st buffer ramm
2012-07-24 23:01 ` Gustavo Padovan
2012-07-25 8:00 ` Malovany, Ram
2012-07-25 8:30 ` Malovany, Ram
2012-08-03 11:40 ` Malovany, Ram
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.