| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210128161029.a53la6e3dv5bzazn@steredhat>
Date: Thu, 28 Jan 2021 17:10:29 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Arseny Krasnov <arseny.krasnov@...persky.com>
Cc: Stefan Hajnoczi <stefanha@...hat.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Andra Paraschiv <andraprs@...zon.com>,
Colin Ian King <colin.king@...onical.com>,
Jeff Vander Stoep <jeffv@...gle.com>, kvm@...r.kernel.org,
virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, stsp2@...dex.ru, oxffffaa@...il.com
Subject: Re: [RFC PATCH v3 01/13] af_vsock: prepare for SOCK_SEQPACKET support
I think the patch title should be more explicit, so something like
vsock: generalize function to manage connectible sockets
On Mon, Jan 25, 2021 at 02:11:28PM +0300, Arseny Krasnov wrote:
>This prepares af_vsock.c for SEQPACKET support:
>1) As both stream and seqpacket sockets are connection oriented, add
> check for SOCK_SEQPACKET to conditions where SOCK_STREAM is checked.
>2) Some functions such as setsockopt(), getsockopt(), connect(),
> recvmsg(), sendmsg() are shared between both types of sockets, so
> rename them in general manner and create entry points for each type
> of socket to call these functions(for stream in this patch, for
> seqpacket in further patches).
>
>Signed-off-by: Arseny Krasnov <arseny.krasnov@...persky.com>
>---
> net/vmw_vsock/af_vsock.c | 91 +++++++++++++++++++++++++++++-----------
> 1 file changed, 67 insertions(+), 24 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index b12d3a322242..c9ce57db9554 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -604,8 +604,8 @@ static void vsock_pending_work(struct work_struct *work)
>
> /**** SOCKET OPERATIONS ****/
>
>-static int __vsock_bind_stream(struct vsock_sock *vsk,
>- struct sockaddr_vm *addr)
>+static int __vsock_bind_connectible(struct vsock_sock *vsk,
>+ struct sockaddr_vm *addr)
> {
> static u32 port;
> struct sockaddr_vm new_addr;
>@@ -685,7 +685,7 @@ static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr)
> switch (sk->sk_socket->type) {
> case SOCK_STREAM:
> spin_lock_bh(&vsock_table_lock);
>- retval = __vsock_bind_stream(vsk, addr);
>+ retval = __vsock_bind_connectible(vsk, addr);
> spin_unlock_bh(&vsock_table_lock);
> break;
>
>@@ -767,6 +767,11 @@ static struct sock *__vsock_create(struct net *net,
> return sk;
> }
>
>+static bool sock_type_connectible(u16 type)
>+{
>+ return (type == SOCK_STREAM || type == SOCK_SEQPACKET);
>+}
>+
I think it's okay to add this function in this patch, but until
SOCK_SEQPACKET is not supported, I would check only SOCK_STREAM and add
SOCK_SEQPACKET only when you add 'vsock_seqpacket_ops' later.
> static void __vsock_release(struct sock *sk, int level)
> {
> if (sk) {
>@@ -785,7 +790,7 @@ static void __vsock_release(struct sock *sk, int level)
>
> if (vsk->transport)
> vsk->transport->release(vsk);
>- else if (sk->sk_type == SOCK_STREAM)
>+ else if (sock_type_connectible(sk->sk_type))
> vsock_remove_sock(vsk);
>
> sock_orphan(sk);
>@@ -945,7 +950,7 @@ static int vsock_shutdown(struct socket *sock, int mode)
> sk = sock->sk;
> if (sock->state == SS_UNCONNECTED) {
> err = -ENOTCONN;
>- if (sk->sk_type == SOCK_STREAM)
>+ if (sock_type_connectible(sk->sk_type))
> return err;
> } else {
> sock->state = SS_DISCONNECTING;
>@@ -960,7 +965,7 @@ static int vsock_shutdown(struct socket *sock, int mode)
> sk->sk_state_change(sk);
> release_sock(sk);
>
>- if (sk->sk_type == SOCK_STREAM) {
>+ if (sock_type_connectible(sk->sk_type)) {
> sock_reset_flag(sk, SOCK_DONE);
> vsock_send_shutdown(sk, mode);
> }
>@@ -1013,7 +1018,7 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
> if (!(sk->sk_shutdown & SEND_SHUTDOWN))
> mask |= EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND;
>
>- } else if (sock->type == SOCK_STREAM) {
>+ } else if (sock_type_connectible(sk->sk_type)) {
> const struct vsock_transport *transport = vsk->transport;
> lock_sock(sk);
>
>@@ -1259,8 +1264,8 @@ static void vsock_connect_timeout(struct work_struct *work)
> sock_put(sk);
> }
>
>-static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
>- int addr_len, int flags)
>+static int vsock_connect(struct socket *sock, struct sockaddr *addr,
>+ int addr_len, int flags)
> {
> int err;
> struct sock *sk;
>@@ -1395,6 +1400,12 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
> return err;
> }
>
>+static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
>+ int addr_len, int flags)
>+{
>+ return vsock_connect(sock, addr, addr_len, flags);
>+}
>+
I think you can directly use vsock_connect in 'vsock_stream_ops'.
> static int vsock_accept(struct socket *sock, struct socket *newsock, int flags,
> bool kern)
> {
>@@ -1410,7 +1421,7 @@ static int vsock_accept(struct socket *sock, struct socket *newsock, int flags,
>
> lock_sock(listener);
>
>- if (sock->type != SOCK_STREAM) {
>+ if (!sock_type_connectible(sock->type)) {
> err = -EOPNOTSUPP;
> goto out;
> }
>@@ -1487,7 +1498,7 @@ static int vsock_listen(struct socket *sock, int
>backlog)
>
> lock_sock(sk);
>
>- if (sock->type != SOCK_STREAM) {
>+ if (!sock_type_connectible(sk->sk_type)) {
> err = -EOPNOTSUPP;
> goto out;
> }
>@@ -1531,11 +1542,11 @@ static void vsock_update_buffer_size(struct vsock_sock *vsk,
> vsk->buffer_size = val;
> }
>
>-static int vsock_stream_setsockopt(struct socket *sock,
>- int level,
>- int optname,
>- sockptr_t optval,
>- unsigned int optlen)
>+static int vsock_connectible_setsockopt(struct socket *sock,
>+ int level,
>+ int optname,
>+ sockptr_t optval,
>+ unsigned int optlen)
> {
> int err;
> struct sock *sk;
>@@ -1612,10 +1623,20 @@ static int vsock_stream_setsockopt(struct socket *sock,
> return err;
> }
>
>-static int vsock_stream_getsockopt(struct socket *sock,
>- int level, int optname,
>- char __user *optval,
>- int __user *optlen)
>+static int vsock_stream_setsockopt(struct socket *sock,
>+ int level,
>+ int optname,
>+ sockptr_t optval,
>+ unsigned int optlen)
>+{
>+ return vsock_connectible_setsockopt(sock, level, optname, optval,
>+ optlen);
>+}
As before, I think you can directly use vsock_connectible_setsockopt in
'vsock_stream_ops'.
>+
>+static int vsock_connectible_getsockopt(struct socket *sock,
>+ int level, int optname,
>+ char __user *optval,
>+ int __user *optlen)
> {
> int err;
> int len;
>@@ -1683,8 +1704,17 @@ static int vsock_stream_getsockopt(struct socket *sock,
> return 0;
> }
>
>-static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
>- size_t len)
>+static int vsock_stream_getsockopt(struct socket *sock,
>+ int level, int optname,
>+ char __user *optval,
>+ int __user *optlen)
>+{
>+ return vsock_connectible_getsockopt(sock, level, optname, optval,
>+ optlen);
>+}
>+
Ditto.
>+static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
>+ size_t len)
> {
> struct sock *sk;
> struct vsock_sock *vsk;
>@@ -1822,10 +1852,16 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
> return err;
> }
>
>+static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
>+ size_t len)
>+{
>+ return vsock_connectible_sendmsg(sock, msg, len);
>+}
>+
Ditto.
>
> static int
>-vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>- int flags)
>+vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>+ int flags)
> {
> struct sock *sk;
> struct vsock_sock *vsk;
>@@ -1995,6 +2031,13 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> return err;
> }
>
>+static int
>+vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>+ int flags)
>+{
>+ return vsock_connectible_recvmsg(sock, msg, len, flags);
>+}
>+
Ditto.
> static const struct proto_ops vsock_stream_ops = {
> .family = PF_VSOCK,
> .owner = THIS_MODULE,
>--
>2.25.1
>
Powered by blists - more mailing lists