netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] Don't attach callback to a going-away netlink socket
       [not found] <46236082.3000101@sw.ru>
@ 2007-04-16 11:41 ` Patrick McHardy
  2007-04-16 11:58   ` Pavel Emelianov
  0 siblings, 1 reply; 3+ messages in thread
From: Patrick McHardy @ 2007-04-16 11:41 UTC (permalink / raw)
  To: Pavel Emelianov
  Cc: Andrew Morton, Linux Kernel Mailing List, devel,
	Linux Netdev List

Pavel Emelianov wrote:
> From: Denis Lunev <den@openvz.org>
> 
> 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.


Can you describe the race in more detail please?


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

* Re: [PATCH] Don't attach callback to a going-away netlink socket
  2007-04-16 11:41 ` [PATCH] Don't attach callback to a going-away netlink socket Patrick McHardy
@ 2007-04-16 11:58   ` Pavel Emelianov
  2007-04-16 12:55     ` Patrick McHardy
  0 siblings, 1 reply; 3+ messages in thread
From: Pavel Emelianov @ 2007-04-16 11:58 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Andrew Morton, Linux Kernel Mailing List, devel,
	Linux Netdev List

Patrick McHardy wrote:
> Pavel Emelianov wrote:
>> From: Denis Lunev <den@openvz.org>
>>
>> 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.
> 
> 
> Can you describe the race in more detail please?
> 
> 

Here it is:

CPU1:                           CPU2
netlink_release():              netlink_dump_start():

                                sk = netlink_lookup(); /* OK */

netlink_remove();

spin_lock(&nlk->cb_lock);
if (nlk->cb) { /* false */
  ...
}
spin_unlock(&nlk->cb_lock);

                                spin_lock(&nlk->cb_lock);
                                if (nlk->cb) { /* false */
                                         ...
                                }
                                nlk->cb = cb;
                                spin_unlock(&nlk->cb_lock);
                                ...
sock_orphan(sk);
/*
 * proceed with releasing
 * the socket
 */

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.

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

* Re: [PATCH] Don't attach callback to a going-away netlink socket
  2007-04-16 11:58   ` Pavel Emelianov
@ 2007-04-16 12:55     ` Patrick McHardy
  0 siblings, 0 replies; 3+ messages in thread
From: Patrick McHardy @ 2007-04-16 12:55 UTC (permalink / raw)
  To: Pavel Emelianov
  Cc: Andrew Morton, Linux Kernel Mailing List, devel,
	Linux Netdev List

Pavel Emelianov wrote:
> Patrick McHardy wrote:
> 
>>>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.
>>
>>
>>Can you describe the race in more detail please?
>>
> 
> Here it is:
> 
> [...]
> 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.


Thanks, good catch. Your patch also looks good.

Acked-by: Patrick McHardy <kaber@trash.net>


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

end of thread, other threads:[~2007-04-16 12:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <46236082.3000101@sw.ru>
2007-04-16 11:41 ` [PATCH] Don't attach callback to a going-away netlink socket Patrick McHardy
2007-04-16 11:58   ` Pavel Emelianov
2007-04-16 12:55     ` Patrick McHardy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).