All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Eric Biggers <ebiggers@kernel.org>,
	netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, Mao Wenan <maowenan@huawei.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Lorenzo Colitti <lorenzo@google.com>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Subject: Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
Date: Fri, 22 Feb 2019 19:08:50 +0000	[thread overview]
Message-ID: <20190222190850.GF2217@ZenIV.linux.org.uk> (raw)
In-Reply-To: <0de89a50-6d5c-1d7b-19f5-9a13465bcebd@gmail.com>

On Fri, Feb 22, 2019 at 10:25:09AM -0800, Eric Dumazet wrote:
> 
> 
> On 02/22/2019 09:57 AM, Eric Biggers wrote:
> 
> > ->setattr() is called under inode_lock(), which __sock_release() also takes.  So
> > the uses of sock->sk are serialized.  See commit 6d8c50dcb029 ("socket: close
> > race condition between sock_close() and sockfs_setattr()").
> 
> Oh right, we added another inode_lock()/inode_unlock() for sock_close()

An interesting question is whether anything else will be confused by
	sock->sk && sock->sk->sk_socket != sock

I'd still like to figure out if we could simply make sock_orphan()
do something like
	if (likely(sk->sk_socket))
		sk->sk_socket->sk = NULL;
just before sk_set_socket(sk, NULL);

That would make for much easier rules; the question is whether anything
relies upon the windows when linkage between socket and sock is not
symmetrical...

  reply	other threads:[~2019-02-22 19:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-21 22:13 [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release() Eric Biggers
2019-02-22  2:51 ` Al Viro
2019-02-22 17:45 ` Eric Dumazet
2019-02-22 17:57   ` Eric Biggers
2019-02-22 18:25     ` Eric Dumazet
2019-02-22 19:08       ` Al Viro [this message]
2019-02-22 18:05 ` Cong Wang
2019-02-25 18:41 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190222190850.GF2217@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=davem@davemloft.net \
    --cc=ebiggers@kernel.org \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo@google.com \
    --cc=maowenan@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.