Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-05-03 Thread David Miller
From: Herbert Xu <[EMAIL PROTECTED]> Date: Wed, 2 May 2007 14:12:22 +1000 > Dave, thanks for reminding me. Here it is. > > [NETLINK]: Kill CB only when socket is unused > > Since we can still receive packets until all references to the > socket are gone, we don't need to kill the CB until that

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-05-03 Thread David Miller
From: Herbert Xu [EMAIL PROTECTED] Date: Wed, 2 May 2007 14:12:22 +1000 Dave, thanks for reminding me. Here it is. [NETLINK]: Kill CB only when socket is unused Since we can still receive packets until all references to the socket are gone, we don't need to kill the CB until that

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-05-01 Thread Herbert Xu
On Sat, Apr 28, 2007 at 11:18:49PM -0700, David Miller wrote: > > Herbert, could you refresh this refinement to the current > tree? Dave, thanks for reminding me. Here it is. [NETLINK]: Kill CB only when socket is unused Since we can still receive packets until all references to the socket

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-05-01 Thread Herbert Xu
On Sat, Apr 28, 2007 at 11:18:49PM -0700, David Miller wrote: Herbert, could you refresh this refinement to the current tree? Dave, thanks for reminding me. Here it is. [NETLINK]: Kill CB only when socket is unused Since we can still receive packets until all references to the socket are

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-29 Thread David Miller
From: Herbert Xu <[EMAIL PROTECTED]> Date: Thu, 19 Apr 2007 12:13:51 +1000 > [NETLINK]: Kill CB only when socket is unused > > Since we can still receive packets until all references to the > socket are gone, we don't need to kill the CB until that happens. > This also aligns ourselves with the

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-29 Thread David Miller
From: Herbert Xu [EMAIL PROTECTED] Date: Thu, 19 Apr 2007 12:13:51 +1000 [NETLINK]: Kill CB only when socket is unused Since we can still receive packets until all references to the socket are gone, we don't need to kill the CB until that happens. This also aligns ourselves with the receive

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Herbert Xu
David Miller <[EMAIL PROTECTED]> wrote: > > As discussed in this thread there might be other ways to a > approach this, but this fix is good for now. > > Patch applied, thank you. Actually I was going to suggest something like this: [NETLINK]: Kill CB only when socket is unused Since we can

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread David Miller
From: Pavel Emelianov <[EMAIL PROTECTED]> Date: Wed, 18 Apr 2007 12:16:18 +0400 > The proposal it to make sock_orphan before detaching the callback > in netlink_release() and to check for the sock to be SOCK_DEAD in > netlink_dump_start() before setting a new callback. As discussed in this

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 11:16:50AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: > > That is what I referred to as error path. Btw, with positive return > > value we end up in subsequent call to input which will free callback > > under lock as expected. > > > No, nothing is going to call

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Patrick McHardy
Evgeniy Polyakov wrote: > On Wed, Apr 18, 2007 at 10:50:42AM +0200, Patrick McHardy ([EMAIL PROTECTED]) > wrote: > >>>I thought that with releasing a socket, which will have a callback >>>attached only results in a leak of the callback? In that case we can >>>just free it in dump() just like it

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 01:03:56PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: > > Yes, you are right, that it will not be freed in netlink_release(), > > but it will be freed in netlink_dump() after it is processed (in no-error > > path only though). > > > > But error path will leak

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 10:50:42AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: > >>It already does (netlink_destroy_callback), but that doesn't help > >>with this race though since without this patch we don't enter the > >>error path. > > > > I thought that with releasing a socket, which

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Pavel Emelianov
Evgeniy Polyakov wrote: > On Wed, Apr 18, 2007 at 12:32:40PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) > wrote: >> Evgeniy Polyakov wrote: >>> On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL >>> PROTECTED]) wrote: Sorry, I forgot to put netdev and David in Cc when I first

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Patrick McHardy
Evgeniy Polyakov wrote: > On Wed, Apr 18, 2007 at 10:26:31AM +0200, Patrick McHardy ([EMAIL PROTECTED]) > wrote: > >>>Out of curiosity, why not to fix a netlink_dump_start() to remove >>>callback in error path, since in 'no-error' path it removes it in >>>netlink_dump(). >> >> >>It already does

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 12:32:40PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: > Evgeniy Polyakov wrote: > > On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL > > PROTECTED]) wrote: > >> Sorry, I forgot to put netdev and David in Cc when I first sent it. > >> > >> There is

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 10:26:31AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: > Evgeniy Polyakov wrote: > > On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL > > PROTECTED]) wrote: > > > >>Sorry, I forgot to put netdev and David in Cc when I first sent it. > >> > >>There

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Patrick McHardy
Evgeniy Polyakov wrote: > On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) > wrote: > >>Sorry, I forgot to put netdev and David in Cc when I first sent it. >> >>There is a race between netlink_dump_start() and netlink_release() >>that can lead to the situation when a

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Pavel Emelianov
Evgeniy Polyakov wrote: > On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) > wrote: >> Sorry, I forgot to put netdev and David in Cc when I first sent it. >> >> There is a race between netlink_dump_start() and netlink_release() >> that can lead to the situation when a

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: > Sorry, I forgot to put netdev and David in Cc when I first sent it. > > There is a race between netlink_dump_start() and netlink_release() > that can lead to the situation when a netlink socket with non-zero >

[NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Pavel Emelianov
Sorry, I forgot to put netdev and David in Cc when I first sent it. There is a race between netlink_dump_start() and netlink_release() that can lead to the situation when a netlink socket with non-zero callback is freed. Here it is: CPU1: CPU2 netlink_release():

[NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Pavel Emelianov
Sorry, I forgot to put netdev and David in Cc when I first sent it. There is a race between netlink_dump_start() and netlink_release() that can lead to the situation when a netlink socket with non-zero callback is freed. Here it is: CPU1: CPU2 netlink_release():

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Sorry, I forgot to put netdev and David in Cc when I first sent it. There is a race between netlink_dump_start() and netlink_release() that can lead to the situation when a netlink socket with non-zero

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Patrick McHardy
Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Sorry, I forgot to put netdev and David in Cc when I first sent it. There is a race between netlink_dump_start() and netlink_release() that can lead to the situation when a netlink

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Pavel Emelianov
Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Sorry, I forgot to put netdev and David in Cc when I first sent it. There is a race between netlink_dump_start() and netlink_release() that can lead to the situation when a netlink

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 10:26:31AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Sorry, I forgot to put netdev and David in Cc when I first sent it. There is a race

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 12:32:40PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Sorry, I forgot to put netdev and David in Cc when I first sent it. There is a race between

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Patrick McHardy
Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 10:26:31AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Out of curiosity, why not to fix a netlink_dump_start() to remove callback in error path, since in 'no-error' path it removes it in netlink_dump(). It already does

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Pavel Emelianov
Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 12:32:40PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 12:16:18PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Sorry, I forgot to put netdev and David in Cc when I first sent it.

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 10:50:42AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: It already does (netlink_destroy_callback), but that doesn't help with this race though since without this patch we don't enter the error path. I thought that with releasing a socket, which will have a

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 01:03:56PM +0400, Pavel Emelianov ([EMAIL PROTECTED]) wrote: Yes, you are right, that it will not be freed in netlink_release(), but it will be freed in netlink_dump() after it is processed (in no-error path only though). But error path will leak it. On

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Patrick McHardy
Evgeniy Polyakov wrote: On Wed, Apr 18, 2007 at 10:50:42AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: I thought that with releasing a socket, which will have a callback attached only results in a leak of the callback? In that case we can just free it in dump() just like it is done in

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Evgeniy Polyakov
On Wed, Apr 18, 2007 at 11:16:50AM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: That is what I referred to as error path. Btw, with positive return value we end up in subsequent call to input which will free callback under lock as expected. No, nothing is going to call

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread David Miller
From: Pavel Emelianov [EMAIL PROTECTED] Date: Wed, 18 Apr 2007 12:16:18 +0400 The proposal it to make sock_orphan before detaching the callback in netlink_release() and to check for the sock to be SOCK_DEAD in netlink_dump_start() before setting a new callback. As discussed in this thread

Re: [NETLINK] Don't attach callback to a going-away netlink socket

2007-04-18 Thread Herbert Xu
David Miller [EMAIL PROTECTED] wrote: As discussed in this thread there might be other ways to a approach this, but this fix is good for now. Patch applied, thank you. Actually I was going to suggest something like this: [NETLINK]: Kill CB only when socket is unused Since we can still