HI Gustavo,
> -----Original Message-----
> From: Malovany, Ram
> Sent: Wednesday, July 25, 2012 11:01 AM
> To: 'Gustavo Padovan'
> Cc: [email protected]
> Subject: RE: [PATCH] Bluetooth: Fix for double free of st buffer.
>
> HI Gustavo,
>
> > -----Original Message-----
> > From: Gustavo Padovan [mailto:[email protected]]
> > Sent: Wednesday, July 25, 2012 2:01 AM
> > To: Malovany, Ram
> > Cc: [email protected]
> > Subject: Re: [PATCH] Bluetooth: Fix for double free of st buffer.
> >
> > Hi Ram,
> >
> > * [email protected] <[email protected]> [2012-07-17 10:49:30 +0300]:
> >
> > > From: Ram Malovany <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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