All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails
@ 2012-07-19  7:24 Jaganath Kanakkassery
  2012-07-19  7:52 ` Andrei Emeltchenko
  2012-07-25  7:51 ` Gustavo Padovan
  0 siblings, 2 replies; 6+ messages in thread
From: Jaganath Kanakkassery @ 2012-07-19  7:24 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jaganath Kanakkassery

If l2cap_chan_create() fails then it will return from l2cap_sock_kill
since zapped flag of sk is reset.

Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
---
 net/bluetooth/l2cap_sock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 79350d1..419857d 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1174,7 +1174,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int p
 
 	chan = l2cap_chan_create();
 	if (!chan) {
-		l2cap_sock_kill(sk);
+		sk_free(sk);
 		return NULL;
 	}
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails
  2012-07-19  7:24 [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails Jaganath Kanakkassery
@ 2012-07-19  7:52 ` Andrei Emeltchenko
  2012-07-19 11:20   ` Jaganath Kanakkassery
  2012-07-25  7:51 ` Gustavo Padovan
  1 sibling, 1 reply; 6+ messages in thread
From: Andrei Emeltchenko @ 2012-07-19  7:52 UTC (permalink / raw)
  To: Jaganath Kanakkassery; +Cc: linux-bluetooth

Hi Jaganath,

On Thu, Jul 19, 2012 at 12:54:04PM +0530, Jaganath Kanakkassery wrote:
> If l2cap_chan_create() fails then it will return from l2cap_sock_kill
> since zapped flag of sk is reset.
> 
> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
> ---
>  net/bluetooth/l2cap_sock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 79350d1..419857d 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1174,7 +1174,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int p
>  
>  	chan = l2cap_chan_create();
>  	if (!chan) {
> -		l2cap_sock_kill(sk);
> +		sk_free(sk);

Could you consider using sock_put which will call sk_free,
maybe we need to add also sock_orphan?

Best regards 
Andrei Emeltchenko 

>  		return NULL;
>  	}
>  
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails
  2012-07-19  7:52 ` Andrei Emeltchenko
@ 2012-07-19 11:20   ` Jaganath Kanakkassery
  2012-07-19 11:40     ` Andrei Emeltchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Jaganath Kanakkassery @ 2012-07-19 11:20 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

--------------------------------------------------
From: "Andrei Emeltchenko" <andrei.emeltchenko.news@gmail.com>
Sent: Thursday, July 19, 2012 1:22 PM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH] Bluetooth: Fix socket not getting freed if l2cap 
channel create fails

> Hi Jaganath,
>
> On Thu, Jul 19, 2012 at 12:54:04PM +0530, Jaganath Kanakkassery wrote:
>> If l2cap_chan_create() fails then it will return from l2cap_sock_kill
>> since zapped flag of sk is reset.
>>
>> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
>> ---
>>  net/bluetooth/l2cap_sock.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>> index 79350d1..419857d 100644
>> --- a/net/bluetooth/l2cap_sock.c
>> +++ b/net/bluetooth/l2cap_sock.c
>> @@ -1174,7 +1174,7 @@ static struct sock *l2cap_sock_alloc(struct net 
>> *net, struct socket *sock, int p
>>
>>  chan = l2cap_chan_create();
>>  if (!chan) {
>> - l2cap_sock_kill(sk);
>> + sk_free(sk);
>
> Could you consider using sock_put which will call sk_free,
> maybe we need to add also sock_orphan?

Ok, Actually I used sk_free since there is not refcount increase at this 
point
and also I found the same code in rfcomm_sock_alloc().
So should I fix it in RFCOMM also?

Thanks,
Jaganath 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails
  2012-07-19 11:20   ` Jaganath Kanakkassery
@ 2012-07-19 11:40     ` Andrei Emeltchenko
  2012-07-19 12:17       ` Jaganath Kanakkassery
  0 siblings, 1 reply; 6+ messages in thread
From: Andrei Emeltchenko @ 2012-07-19 11:40 UTC (permalink / raw)
  To: Jaganath Kanakkassery; +Cc: linux-bluetooth, Johan Hedberg, Gustavo Padovan

Hi Jaganath,

On Thu, Jul 19, 2012 at 04:50:16PM +0530, Jaganath Kanakkassery wrote:
> Hi Andrei,
> 
> --------------------------------------------------
> From: "Andrei Emeltchenko" <andrei.emeltchenko.news@gmail.com>
> Sent: Thursday, July 19, 2012 1:22 PM
> To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
> Cc: <linux-bluetooth@vger.kernel.org>
> Subject: Re: [PATCH] Bluetooth: Fix socket not getting freed if
> l2cap channel create fails
> 
> >Hi Jaganath,
> >
> >On Thu, Jul 19, 2012 at 12:54:04PM +0530, Jaganath Kanakkassery wrote:
> >>If l2cap_chan_create() fails then it will return from l2cap_sock_kill
> >>since zapped flag of sk is reset.
> >>
> >>Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
> >>---
> >> net/bluetooth/l2cap_sock.c |    2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >>diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> >>index 79350d1..419857d 100644
> >>--- a/net/bluetooth/l2cap_sock.c
> >>+++ b/net/bluetooth/l2cap_sock.c
> >>@@ -1174,7 +1174,7 @@ static struct sock
> >>*l2cap_sock_alloc(struct net *net, struct socket *sock, int p
> >>
> >> chan = l2cap_chan_create();
> >> if (!chan) {
> >>- l2cap_sock_kill(sk);
> >>+ sk_free(sk);
> >
> >Could you consider using sock_put which will call sk_free,
> >maybe we need to add also sock_orphan?
> 
> Ok, Actually I used sk_free since there is not refcount increase at
> this point

Have you tested it? It shall be 1, set by sock_init_data.

> and also I found the same code in rfcomm_sock_alloc().
> So should I fix it in RFCOMM also?

I think using sock_put would be the right approach. Maybe maintainers
could comment here?

Best regards 
Andrei Emeltchenko 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails
  2012-07-19 11:40     ` Andrei Emeltchenko
@ 2012-07-19 12:17       ` Jaganath Kanakkassery
  0 siblings, 0 replies; 6+ messages in thread
From: Jaganath Kanakkassery @ 2012-07-19 12:17 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth, Johan Hedberg, Gustavo Padovan

Hi Andrei,

--------------------------------------------------
From: "Andrei Emeltchenko" <andrei.emeltchenko.news@gmail.com>
Sent: Thursday, July 19, 2012 5:10 PM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>; "Johan Hedberg" 
<johan.hedberg@intel.com>; "Gustavo Padovan" <gustavo@padovan.org>
Subject: Re: [PATCH] Bluetooth: Fix socket not getting freed if l2cap 
channel create fails

> Hi Jaganath,
>
> On Thu, Jul 19, 2012 at 04:50:16PM +0530, Jaganath Kanakkassery wrote:
>> Hi Andrei,
>>
>> --------------------------------------------------
>> From: "Andrei Emeltchenko" <andrei.emeltchenko.news@gmail.com>
>> Sent: Thursday, July 19, 2012 1:22 PM
>> To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
>> Cc: <linux-bluetooth@vger.kernel.org>
>> Subject: Re: [PATCH] Bluetooth: Fix socket not getting freed if
>> l2cap channel create fails
>>
>> >Hi Jaganath,
>> >
>> >On Thu, Jul 19, 2012 at 12:54:04PM +0530, Jaganath Kanakkassery wrote:
>> >>If l2cap_chan_create() fails then it will return from l2cap_sock_kill
>> >>since zapped flag of sk is reset.
>> >>
>> >>Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
>> >>---
>> >> net/bluetooth/l2cap_sock.c |    2 +-
>> >> 1 files changed, 1 insertions(+), 1 deletions(-)
>> >>
>> >>diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>> >>index 79350d1..419857d 100644
>> >>--- a/net/bluetooth/l2cap_sock.c
>> >>+++ b/net/bluetooth/l2cap_sock.c
>> >>@@ -1174,7 +1174,7 @@ static struct sock
>> >>*l2cap_sock_alloc(struct net *net, struct socket *sock, int p
>> >>
>> >> chan = l2cap_chan_create();
>> >> if (!chan) {
>> >>- l2cap_sock_kill(sk);
>> >>+ sk_free(sk);
>> >
>> >Could you consider using sock_put which will call sk_free,
>> >maybe we need to add also sock_orphan?
>>
>> Ok, Actually I used sk_free since there is not refcount increase at
>> this point
>
> Have you tested it? It shall be 1, set by sock_init_data.

Yes it is 1. So even if we use sock_put() , it will decrement the refcount
and call sk_free().

>> and also I found the same code in rfcomm_sock_alloc().
>> So should I fix it in RFCOMM also?
>
> I think using sock_put would be the right approach. Maybe maintainers
> could comment here?

Ok, I will wait for maintainers comments.

Thanks,
Jaganath 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails
  2012-07-19  7:24 [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails Jaganath Kanakkassery
  2012-07-19  7:52 ` Andrei Emeltchenko
@ 2012-07-25  7:51 ` Gustavo Padovan
  1 sibling, 0 replies; 6+ messages in thread
From: Gustavo Padovan @ 2012-07-25  7:51 UTC (permalink / raw)
  To: Jaganath Kanakkassery; +Cc: linux-bluetooth

Hi Jaganath,

* Jaganath Kanakkassery <jaganath.k@samsung.com> [2012-07-19 12:54:04 +0530]:

> If l2cap_chan_create() fails then it will return from l2cap_sock_kill
> since zapped flag of sk is reset.
> 
> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
> ---
>  net/bluetooth/l2cap_sock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Patch has been applied to bluetooth.git. Thanks.

	Gustavo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-07-25  7:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-19  7:24 [PATCH] Bluetooth: Fix socket not getting freed if l2cap channel create fails Jaganath Kanakkassery
2012-07-19  7:52 ` Andrei Emeltchenko
2012-07-19 11:20   ` Jaganath Kanakkassery
2012-07-19 11:40     ` Andrei Emeltchenko
2012-07-19 12:17       ` Jaganath Kanakkassery
2012-07-25  7:51 ` Gustavo Padovan

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.