From: Al Viro <viro@zeniv.linux.org.uk>
To: Tony Lu <tonylu@linux.alibaba.com>
Cc: kgraul@linux.ibm.com, kuba@kernel.org, davem@davemloft.net,
netdev@vger.kernel.org, linux-s390@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH net-next] net/smc: Introduce TCP ULP support
Date: Thu, 4 Aug 2022 03:56:11 +0100 [thread overview]
Message-ID: <Yus1SycZxcd+wHwz@ZenIV> (raw)
In-Reply-To: <20211228134435.41774-1-tonylu@linux.alibaba.com>
Half a year too late, but then it hadn't been posted on fsdevel.
Which it really should have been, due to
> + /* replace tcp socket to smc */
> + smcsock->file = tcp->file;
> + smcsock->file->private_data = smcsock;
> + smcsock->file->f_inode = SOCK_INODE(smcsock); /* replace inode when sock_close */
> + smcsock->file->f_path.dentry->d_inode = SOCK_INODE(smcsock); /* dput() in __fput */
> + tcp->file = NULL;
this. It violates a bunch of rather fundamental assertions about the
data structures you are playing with, and I'm not even going into the
lifetime and refcounting issues.
* ->d_inode of a busy positive dentry never changes while refcount
of dentry remains positive. A lot of places in VFS rely upon that.
* ->f_inode of a file never changes, period.
* ->private_data of a struct file associated with a socket never
changes; it can be accessed lockless, with no precautions beyond "make sure
that refcount of struct file will remain positive".
PS: more than one thread could be calling methods of that struct socket at the
same time; what's to stop e.g. connect(2) on the same sucker (e.g. called on
the same descriptor from a different thread that happens to share the same
descriptor table) to be sitting there trying to lock the struct sock currently
held locked by caller of tcp_set_ulp()?
next prev parent reply other threads:[~2022-08-04 2:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-28 13:44 [PATCH net-next] net/smc: Introduce TCP ULP support Tony Lu
2021-12-30 15:03 ` Karsten Graul
2021-12-31 9:13 ` Tony Lu
2022-01-03 9:09 ` Karsten Graul
2022-01-02 12:20 ` patchwork-bot+netdevbpf
2022-08-04 2:56 ` Al Viro [this message]
2022-08-04 3:48 ` Al Viro
2022-08-11 9:29 ` Tony Lu
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=Yus1SycZxcd+wHwz@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=davem@davemloft.net \
--cc=kgraul@linux.ibm.com \
--cc=kuba@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=tonylu@linux.alibaba.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.