linux-sctp.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect
@ 2018-05-20  8:39 Xin Long
  2018-05-21 11:46 ` Neil Horman
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Xin Long @ 2018-05-20  8:39 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Neil Horman, mkubecek

Now sctp uses inet_dgram_connect as its proto_ops .connect, and the flags
param can't be passed into its proto .connect where this flags is really
needed.

sctp works around it by getting flags from socket file in __sctp_connect.
It works for connecting from userspace, as inherently the user sock has
socket file and it passes f_flags as the flags param into the proto_ops
.connect.

However, the sock created by sock_create_kern doesn't have a socket file,
and it passes the flags (like O_NONBLOCK) by using the flags param in
kernel_connect, which calls proto_ops .connect later.

So to fix it, this patch defines a new proto_ops .connect for sctp,
sctp_inet_connect, which calls __sctp_connect() directly with this
flags param. After this, the sctp's proto .connect can be removed.

Note that sctp_inet_connect doesn't need to do some checks that are not
needed for sctp, which makes thing better than with inet_dgram_connect.

Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/sctp.h |  2 ++
 net/sctp/ipv6.c         |  2 +-
 net/sctp/protocol.c     |  2 +-
 net/sctp/socket.c       | 51 +++++++++++++++++++++++++++++++++----------------
 4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 28b996d..35498e6 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -103,6 +103,8 @@ void sctp_addr_wq_mgmt(struct net *, struct sctp_sockaddr_entry *, int);
 /*
  * sctp/socket.c
  */
+int sctp_inet_connect(struct socket *sock, struct sockaddr *uaddr,
+		      int addr_len, int flags);
 int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb);
 int sctp_inet_listen(struct socket *sock, int backlog);
 void sctp_write_space(struct sock *sk);
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 4224711..0cd2e76 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -1006,7 +1006,7 @@ static const struct proto_ops inet6_seqpacket_ops = {
 	.owner		   = THIS_MODULE,
 	.release	   = inet6_release,
 	.bind		   = inet6_bind,
-	.connect	   = inet_dgram_connect,
+	.connect	   = sctp_inet_connect,
 	.socketpair	   = sock_no_socketpair,
 	.accept		   = inet_accept,
 	.getname	   = sctp_getname,
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index d685f84..6bf0a99 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1012,7 +1012,7 @@ static const struct proto_ops inet_seqpacket_ops = {
 	.owner		   = THIS_MODULE,
 	.release	   = inet_release,	/* Needs to be wrapped... */
 	.bind		   = inet_bind,
-	.connect	   = inet_dgram_connect,
+	.connect	   = sctp_inet_connect,
 	.socketpair	   = sock_no_socketpair,
 	.accept		   = inet_accept,
 	.getname	   = inet_getname,	/* Semantics are different.  */
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 80835ac..ae7e7c6 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1086,7 +1086,7 @@ static int sctp_setsockopt_bindx(struct sock *sk,
  */
 static int __sctp_connect(struct sock *sk,
 			  struct sockaddr *kaddrs,
-			  int addrs_size,
+			  int addrs_size, int flags,
 			  sctp_assoc_t *assoc_id)
 {
 	struct net *net = sock_net(sk);
@@ -1104,7 +1104,6 @@ static int __sctp_connect(struct sock *sk,
 	union sctp_addr *sa_addr = NULL;
 	void *addr_buf;
 	unsigned short port;
-	unsigned int f_flags = 0;
 
 	sp = sctp_sk(sk);
 	ep = sp->ep;
@@ -1254,13 +1253,7 @@ static int __sctp_connect(struct sock *sk,
 	sp->pf->to_sk_daddr(sa_addr, sk);
 	sk->sk_err = 0;
 
-	/* in-kernel sockets don't generally have a file allocated to them
-	 * if all they do is call sock_create_kern().
-	 */
-	if (sk->sk_socket->file)
-		f_flags = sk->sk_socket->file->f_flags;
-
-	timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
+	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
 
 	if (assoc_id)
 		*assoc_id = asoc->assoc_id;
@@ -1348,7 +1341,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
 				      sctp_assoc_t *assoc_id)
 {
 	struct sockaddr *kaddrs;
-	int err = 0;
+	int err = 0, flags = 0;
 
 	pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
 		 __func__, sk, addrs, addrs_size);
@@ -1367,7 +1360,13 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
 	if (err)
 		goto out_free;
 
-	err = __sctp_connect(sk, kaddrs, addrs_size, assoc_id);
+	/* in-kernel sockets don't generally have a file allocated to them
+	 * if all they do is call sock_create_kern().
+	 */
+	if (sk->sk_socket->file)
+		flags = sk->sk_socket->file->f_flags;
+
+	err = __sctp_connect(sk, kaddrs, addrs_size, flags, assoc_id);
 
 out_free:
 	kvfree(kaddrs);
@@ -4397,16 +4396,26 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
  * len: the size of the address.
  */
 static int sctp_connect(struct sock *sk, struct sockaddr *addr,
-			int addr_len)
+			int addr_len, int flags)
 {
-	int err = 0;
+	struct inet_sock *inet = inet_sk(sk);
 	struct sctp_af *af;
+	int err = 0;
 
 	lock_sock(sk);
 
 	pr_debug("%s: sk:%p, sockaddr:%p, addr_len:%d\n", __func__, sk,
 		 addr, addr_len);
 
+	/* We may need to bind the socket. */
+	if (!inet->inet_num) {
+		if (sk->sk_prot->get_port(sk, 0)) {
+			release_sock(sk);
+			return -EAGAIN;
+		}
+		inet->inet_sport = htons(inet->inet_num);
+	}
+
 	/* Validate addr_len before calling common connect/connectx routine. */
 	af = sctp_get_af_specific(addr->sa_family);
 	if (!af || addr_len < af->sockaddr_len) {
@@ -4415,13 +4424,25 @@ static int sctp_connect(struct sock *sk, struct sockaddr *addr,
 		/* Pass correct addr len to common routine (so it knows there
 		 * is only one address being passed.
 		 */
-		err = __sctp_connect(sk, addr, af->sockaddr_len, NULL);
+		err = __sctp_connect(sk, addr, af->sockaddr_len, flags, NULL);
 	}
 
 	release_sock(sk);
 	return err;
 }
 
+int sctp_inet_connect(struct socket *sock, struct sockaddr *uaddr,
+		      int addr_len, int flags)
+{
+	if (addr_len < sizeof(uaddr->sa_family))
+		return -EINVAL;
+
+	if (uaddr->sa_family = AF_UNSPEC)
+		return -EOPNOTSUPP;
+
+	return sctp_connect(sock->sk, uaddr, addr_len, flags);
+}
+
 /* FIXME: Write comments. */
 static int sctp_disconnect(struct sock *sk, int flags)
 {
@@ -8724,7 +8745,6 @@ struct proto sctp_prot = {
 	.name        =	"SCTP",
 	.owner       =	THIS_MODULE,
 	.close       =	sctp_close,
-	.connect     =	sctp_connect,
 	.disconnect  =	sctp_disconnect,
 	.accept      =	sctp_accept,
 	.ioctl       =	sctp_ioctl,
@@ -8767,7 +8787,6 @@ struct proto sctpv6_prot = {
 	.name		= "SCTPv6",
 	.owner		= THIS_MODULE,
 	.close		= sctp_close,
-	.connect	= sctp_connect,
 	.disconnect	= sctp_disconnect,
 	.accept		= sctp_accept,
 	.ioctl		= sctp_ioctl,
-- 
2.1.0


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

* Re: [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect
  2018-05-20  8:39 [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect Xin Long
@ 2018-05-21 11:46 ` Neil Horman
  2018-05-21 15:52 ` Marcelo Ricardo Leitner
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Neil Horman @ 2018-05-21 11:46 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner, mkubecek

On Sun, May 20, 2018 at 04:39:10PM +0800, Xin Long wrote:
> Now sctp uses inet_dgram_connect as its proto_ops .connect, and the flags
> param can't be passed into its proto .connect where this flags is really
> needed.
> 
> sctp works around it by getting flags from socket file in __sctp_connect.
> It works for connecting from userspace, as inherently the user sock has
> socket file and it passes f_flags as the flags param into the proto_ops
> .connect.
> 
> However, the sock created by sock_create_kern doesn't have a socket file,
> and it passes the flags (like O_NONBLOCK) by using the flags param in
> kernel_connect, which calls proto_ops .connect later.
> 
> So to fix it, this patch defines a new proto_ops .connect for sctp,
> sctp_inet_connect, which calls __sctp_connect() directly with this
> flags param. After this, the sctp's proto .connect can be removed.
> 
> Note that sctp_inet_connect doesn't need to do some checks that are not
> needed for sctp, which makes thing better than with inet_dgram_connect.
> 
> Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/net/sctp/sctp.h |  2 ++
>  net/sctp/ipv6.c         |  2 +-
>  net/sctp/protocol.c     |  2 +-
>  net/sctp/socket.c       | 51 +++++++++++++++++++++++++++++++++----------------
>  4 files changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 28b996d..35498e6 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -103,6 +103,8 @@ void sctp_addr_wq_mgmt(struct net *, struct sctp_sockaddr_entry *, int);
>  /*
>   * sctp/socket.c
>   */
> +int sctp_inet_connect(struct socket *sock, struct sockaddr *uaddr,
> +		      int addr_len, int flags);
>  int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb);
>  int sctp_inet_listen(struct socket *sock, int backlog);
>  void sctp_write_space(struct sock *sk);
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 4224711..0cd2e76 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -1006,7 +1006,7 @@ static const struct proto_ops inet6_seqpacket_ops = {
>  	.owner		   = THIS_MODULE,
>  	.release	   = inet6_release,
>  	.bind		   = inet6_bind,
> -	.connect	   = inet_dgram_connect,
> +	.connect	   = sctp_inet_connect,
>  	.socketpair	   = sock_no_socketpair,
>  	.accept		   = inet_accept,
>  	.getname	   = sctp_getname,
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index d685f84..6bf0a99 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1012,7 +1012,7 @@ static const struct proto_ops inet_seqpacket_ops = {
>  	.owner		   = THIS_MODULE,
>  	.release	   = inet_release,	/* Needs to be wrapped... */
>  	.bind		   = inet_bind,
> -	.connect	   = inet_dgram_connect,
> +	.connect	   = sctp_inet_connect,
>  	.socketpair	   = sock_no_socketpair,
>  	.accept		   = inet_accept,
>  	.getname	   = inet_getname,	/* Semantics are different.  */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 80835ac..ae7e7c6 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1086,7 +1086,7 @@ static int sctp_setsockopt_bindx(struct sock *sk,
>   */
>  static int __sctp_connect(struct sock *sk,
>  			  struct sockaddr *kaddrs,
> -			  int addrs_size,
> +			  int addrs_size, int flags,
>  			  sctp_assoc_t *assoc_id)
>  {
>  	struct net *net = sock_net(sk);
> @@ -1104,7 +1104,6 @@ static int __sctp_connect(struct sock *sk,
>  	union sctp_addr *sa_addr = NULL;
>  	void *addr_buf;
>  	unsigned short port;
> -	unsigned int f_flags = 0;
>  
>  	sp = sctp_sk(sk);
>  	ep = sp->ep;
> @@ -1254,13 +1253,7 @@ static int __sctp_connect(struct sock *sk,
>  	sp->pf->to_sk_daddr(sa_addr, sk);
>  	sk->sk_err = 0;
>  
> -	/* in-kernel sockets don't generally have a file allocated to them
> -	 * if all they do is call sock_create_kern().
> -	 */
> -	if (sk->sk_socket->file)
> -		f_flags = sk->sk_socket->file->f_flags;
> -
> -	timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
> +	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
>  
>  	if (assoc_id)
>  		*assoc_id = asoc->assoc_id;
> @@ -1348,7 +1341,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
>  				      sctp_assoc_t *assoc_id)
>  {
>  	struct sockaddr *kaddrs;
> -	int err = 0;
> +	int err = 0, flags = 0;
>  
>  	pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
>  		 __func__, sk, addrs, addrs_size);
> @@ -1367,7 +1360,13 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
>  	if (err)
>  		goto out_free;
>  
> -	err = __sctp_connect(sk, kaddrs, addrs_size, assoc_id);
> +	/* in-kernel sockets don't generally have a file allocated to them
> +	 * if all they do is call sock_create_kern().
> +	 */
> +	if (sk->sk_socket->file)
> +		flags = sk->sk_socket->file->f_flags;
> +
> +	err = __sctp_connect(sk, kaddrs, addrs_size, flags, assoc_id);
>  
>  out_free:
>  	kvfree(kaddrs);
> @@ -4397,16 +4396,26 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
>   * len: the size of the address.
>   */
>  static int sctp_connect(struct sock *sk, struct sockaddr *addr,
> -			int addr_len)
> +			int addr_len, int flags)
>  {
> -	int err = 0;
> +	struct inet_sock *inet = inet_sk(sk);
>  	struct sctp_af *af;
> +	int err = 0;
>  
>  	lock_sock(sk);
>  
>  	pr_debug("%s: sk:%p, sockaddr:%p, addr_len:%d\n", __func__, sk,
>  		 addr, addr_len);
>  
> +	/* We may need to bind the socket. */
> +	if (!inet->inet_num) {
> +		if (sk->sk_prot->get_port(sk, 0)) {
> +			release_sock(sk);
> +			return -EAGAIN;
> +		}
> +		inet->inet_sport = htons(inet->inet_num);
> +	}
> +
>  	/* Validate addr_len before calling common connect/connectx routine. */
>  	af = sctp_get_af_specific(addr->sa_family);
>  	if (!af || addr_len < af->sockaddr_len) {
> @@ -4415,13 +4424,25 @@ static int sctp_connect(struct sock *sk, struct sockaddr *addr,
>  		/* Pass correct addr len to common routine (so it knows there
>  		 * is only one address being passed.
>  		 */
> -		err = __sctp_connect(sk, addr, af->sockaddr_len, NULL);
> +		err = __sctp_connect(sk, addr, af->sockaddr_len, flags, NULL);
>  	}
>  
>  	release_sock(sk);
>  	return err;
>  }
>  
> +int sctp_inet_connect(struct socket *sock, struct sockaddr *uaddr,
> +		      int addr_len, int flags)
> +{
> +	if (addr_len < sizeof(uaddr->sa_family))
> +		return -EINVAL;
> +
> +	if (uaddr->sa_family = AF_UNSPEC)
> +		return -EOPNOTSUPP;
> +
> +	return sctp_connect(sock->sk, uaddr, addr_len, flags);
> +}
> +
>  /* FIXME: Write comments. */
>  static int sctp_disconnect(struct sock *sk, int flags)
>  {
> @@ -8724,7 +8745,6 @@ struct proto sctp_prot = {
>  	.name        =	"SCTP",
>  	.owner       =	THIS_MODULE,
>  	.close       =	sctp_close,
> -	.connect     =	sctp_connect,
>  	.disconnect  =	sctp_disconnect,
>  	.accept      =	sctp_accept,
>  	.ioctl       =	sctp_ioctl,
> @@ -8767,7 +8787,6 @@ struct proto sctpv6_prot = {
>  	.name		= "SCTPv6",
>  	.owner		= THIS_MODULE,
>  	.close		= sctp_close,
> -	.connect	= sctp_connect,
>  	.disconnect	= sctp_disconnect,
>  	.accept		= sctp_accept,
>  	.ioctl		= sctp_ioctl,
> -- 
> 2.1.0
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>


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

* Re: [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect
  2018-05-20  8:39 [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect Xin Long
  2018-05-21 11:46 ` Neil Horman
@ 2018-05-21 15:52 ` Marcelo Ricardo Leitner
  2018-05-22 12:38 ` Michal Kubecek
  2018-05-22 17:40 ` David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-05-21 15:52 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman, mkubecek

On Sun, May 20, 2018 at 04:39:10PM +0800, Xin Long wrote:
> Now sctp uses inet_dgram_connect as its proto_ops .connect, and the flags
> param can't be passed into its proto .connect where this flags is really
> needed.
> 
> sctp works around it by getting flags from socket file in __sctp_connect.
> It works for connecting from userspace, as inherently the user sock has
> socket file and it passes f_flags as the flags param into the proto_ops
> .connect.
> 
> However, the sock created by sock_create_kern doesn't have a socket file,
> and it passes the flags (like O_NONBLOCK) by using the flags param in
> kernel_connect, which calls proto_ops .connect later.
> 
> So to fix it, this patch defines a new proto_ops .connect for sctp,
> sctp_inet_connect, which calls __sctp_connect() directly with this
> flags param. After this, the sctp's proto .connect can be removed.
> 
> Note that sctp_inet_connect doesn't need to do some checks that are not
> needed for sctp, which makes thing better than with inet_dgram_connect.
> 
> Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/net/sctp/sctp.h |  2 ++
>  net/sctp/ipv6.c         |  2 +-
>  net/sctp/protocol.c     |  2 +-
>  net/sctp/socket.c       | 51 +++++++++++++++++++++++++++++++++----------------
>  4 files changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 28b996d..35498e6 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -103,6 +103,8 @@ void sctp_addr_wq_mgmt(struct net *, struct sctp_sockaddr_entry *, int);
>  /*
>   * sctp/socket.c
>   */
> +int sctp_inet_connect(struct socket *sock, struct sockaddr *uaddr,
> +		      int addr_len, int flags);
>  int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb);
>  int sctp_inet_listen(struct socket *sock, int backlog);
>  void sctp_write_space(struct sock *sk);
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 4224711..0cd2e76 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -1006,7 +1006,7 @@ static const struct proto_ops inet6_seqpacket_ops = {
>  	.owner		   = THIS_MODULE,
>  	.release	   = inet6_release,
>  	.bind		   = inet6_bind,
> -	.connect	   = inet_dgram_connect,
> +	.connect	   = sctp_inet_connect,
>  	.socketpair	   = sock_no_socketpair,
>  	.accept		   = inet_accept,
>  	.getname	   = sctp_getname,
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index d685f84..6bf0a99 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1012,7 +1012,7 @@ static const struct proto_ops inet_seqpacket_ops = {
>  	.owner		   = THIS_MODULE,
>  	.release	   = inet_release,	/* Needs to be wrapped... */
>  	.bind		   = inet_bind,
> -	.connect	   = inet_dgram_connect,
> +	.connect	   = sctp_inet_connect,
>  	.socketpair	   = sock_no_socketpair,
>  	.accept		   = inet_accept,
>  	.getname	   = inet_getname,	/* Semantics are different.  */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 80835ac..ae7e7c6 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1086,7 +1086,7 @@ static int sctp_setsockopt_bindx(struct sock *sk,
>   */
>  static int __sctp_connect(struct sock *sk,
>  			  struct sockaddr *kaddrs,
> -			  int addrs_size,
> +			  int addrs_size, int flags,
>  			  sctp_assoc_t *assoc_id)
>  {
>  	struct net *net = sock_net(sk);
> @@ -1104,7 +1104,6 @@ static int __sctp_connect(struct sock *sk,
>  	union sctp_addr *sa_addr = NULL;
>  	void *addr_buf;
>  	unsigned short port;
> -	unsigned int f_flags = 0;
>  
>  	sp = sctp_sk(sk);
>  	ep = sp->ep;
> @@ -1254,13 +1253,7 @@ static int __sctp_connect(struct sock *sk,
>  	sp->pf->to_sk_daddr(sa_addr, sk);
>  	sk->sk_err = 0;
>  
> -	/* in-kernel sockets don't generally have a file allocated to them
> -	 * if all they do is call sock_create_kern().
> -	 */
> -	if (sk->sk_socket->file)
> -		f_flags = sk->sk_socket->file->f_flags;
> -
> -	timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
> +	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
>  
>  	if (assoc_id)
>  		*assoc_id = asoc->assoc_id;
> @@ -1348,7 +1341,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
>  				      sctp_assoc_t *assoc_id)
>  {
>  	struct sockaddr *kaddrs;
> -	int err = 0;
> +	int err = 0, flags = 0;
>  
>  	pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
>  		 __func__, sk, addrs, addrs_size);
> @@ -1367,7 +1360,13 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
>  	if (err)
>  		goto out_free;
>  
> -	err = __sctp_connect(sk, kaddrs, addrs_size, assoc_id);
> +	/* in-kernel sockets don't generally have a file allocated to them
> +	 * if all they do is call sock_create_kern().
> +	 */
> +	if (sk->sk_socket->file)
> +		flags = sk->sk_socket->file->f_flags;
> +
> +	err = __sctp_connect(sk, kaddrs, addrs_size, flags, assoc_id);
>  
>  out_free:
>  	kvfree(kaddrs);
> @@ -4397,16 +4396,26 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
>   * len: the size of the address.
>   */
>  static int sctp_connect(struct sock *sk, struct sockaddr *addr,
> -			int addr_len)
> +			int addr_len, int flags)
>  {
> -	int err = 0;
> +	struct inet_sock *inet = inet_sk(sk);
>  	struct sctp_af *af;
> +	int err = 0;
>  
>  	lock_sock(sk);
>  
>  	pr_debug("%s: sk:%p, sockaddr:%p, addr_len:%d\n", __func__, sk,
>  		 addr, addr_len);
>  
> +	/* We may need to bind the socket. */
> +	if (!inet->inet_num) {
> +		if (sk->sk_prot->get_port(sk, 0)) {
> +			release_sock(sk);
> +			return -EAGAIN;
> +		}
> +		inet->inet_sport = htons(inet->inet_num);
> +	}
> +
>  	/* Validate addr_len before calling common connect/connectx routine. */
>  	af = sctp_get_af_specific(addr->sa_family);
>  	if (!af || addr_len < af->sockaddr_len) {
> @@ -4415,13 +4424,25 @@ static int sctp_connect(struct sock *sk, struct sockaddr *addr,
>  		/* Pass correct addr len to common routine (so it knows there
>  		 * is only one address being passed.
>  		 */
> -		err = __sctp_connect(sk, addr, af->sockaddr_len, NULL);
> +		err = __sctp_connect(sk, addr, af->sockaddr_len, flags, NULL);
>  	}
>  
>  	release_sock(sk);
>  	return err;
>  }
>  
> +int sctp_inet_connect(struct socket *sock, struct sockaddr *uaddr,
> +		      int addr_len, int flags)
> +{
> +	if (addr_len < sizeof(uaddr->sa_family))
> +		return -EINVAL;
> +
> +	if (uaddr->sa_family = AF_UNSPEC)
> +		return -EOPNOTSUPP;
> +
> +	return sctp_connect(sock->sk, uaddr, addr_len, flags);
> +}
> +
>  /* FIXME: Write comments. */
>  static int sctp_disconnect(struct sock *sk, int flags)
>  {
> @@ -8724,7 +8745,6 @@ struct proto sctp_prot = {
>  	.name        =	"SCTP",
>  	.owner       =	THIS_MODULE,
>  	.close       =	sctp_close,
> -	.connect     =	sctp_connect,
>  	.disconnect  =	sctp_disconnect,
>  	.accept      =	sctp_accept,
>  	.ioctl       =	sctp_ioctl,
> @@ -8767,7 +8787,6 @@ struct proto sctpv6_prot = {
>  	.name		= "SCTPv6",
>  	.owner		= THIS_MODULE,
>  	.close		= sctp_close,
> -	.connect	= sctp_connect,
>  	.disconnect	= sctp_disconnect,
>  	.accept		= sctp_accept,
>  	.ioctl		= sctp_ioctl,
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect
  2018-05-20  8:39 [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect Xin Long
  2018-05-21 11:46 ` Neil Horman
  2018-05-21 15:52 ` Marcelo Ricardo Leitner
@ 2018-05-22 12:38 ` Michal Kubecek
  2018-05-22 17:40 ` David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Michal Kubecek @ 2018-05-22 12:38 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner, Neil Horman

On Sun, May 20, 2018 at 04:39:10PM +0800, Xin Long wrote:
> Now sctp uses inet_dgram_connect as its proto_ops .connect, and the flags
> param can't be passed into its proto .connect where this flags is really
> needed.
> 
> sctp works around it by getting flags from socket file in __sctp_connect.
> It works for connecting from userspace, as inherently the user sock has
> socket file and it passes f_flags as the flags param into the proto_ops
> .connect.
> 
> However, the sock created by sock_create_kern doesn't have a socket file,
> and it passes the flags (like O_NONBLOCK) by using the flags param in
> kernel_connect, which calls proto_ops .connect later.
> 
> So to fix it, this patch defines a new proto_ops .connect for sctp,
> sctp_inet_connect, which calls __sctp_connect() directly with this
> flags param. After this, the sctp's proto .connect can be removed.
> 
> Note that sctp_inet_connect doesn't need to do some checks that are not
> needed for sctp, which makes thing better than with inet_dgram_connect.
> 
> Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

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

* Re: [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect
  2018-05-20  8:39 [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect Xin Long
                   ` (2 preceding siblings ...)
  2018-05-22 12:38 ` Michal Kubecek
@ 2018-05-22 17:40 ` David Miller
  2018-05-23  6:55   ` Xin Long
  3 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2018-05-22 17:40 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, mkubecek

From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 20 May 2018 16:39:10 +0800

> Now sctp uses inet_dgram_connect as its proto_ops .connect, and the flags
> param can't be passed into its proto .connect where this flags is really
> needed.
> 
> sctp works around it by getting flags from socket file in __sctp_connect.
> It works for connecting from userspace, as inherently the user sock has
> socket file and it passes f_flags as the flags param into the proto_ops
> .connect.
> 
> However, the sock created by sock_create_kern doesn't have a socket file,
> and it passes the flags (like O_NONBLOCK) by using the flags param in
> kernel_connect, which calls proto_ops .connect later.
> 
> So to fix it, this patch defines a new proto_ops .connect for sctp,
> sctp_inet_connect, which calls __sctp_connect() directly with this
> flags param. After this, the sctp's proto .connect can be removed.
> 
> Note that sctp_inet_connect doesn't need to do some checks that are not
> needed for sctp, which makes thing better than with inet_dgram_connect.
> 
> Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied, thank you.

I don't see a Fixes: tag, please give me some guidance me wrt. -stable.

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

* Re: [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect
  2018-05-22 17:40 ` David Miller
@ 2018-05-23  6:55   ` Xin Long
  0 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2018-05-23  6:55 UTC (permalink / raw)
  To: David Miller
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, Neil Horman,
	Michal Kubecek

On Wed, May 23, 2018 at 1:40 AM, David Miller <davem@davemloft.net> wrote:
> From: Xin Long <lucien.xin@gmail.com>
> Date: Sun, 20 May 2018 16:39:10 +0800
>
>> Now sctp uses inet_dgram_connect as its proto_ops .connect, and the flags
>> param can't be passed into its proto .connect where this flags is really
>> needed.
>>
>> sctp works around it by getting flags from socket file in __sctp_connect.
>> It works for connecting from userspace, as inherently the user sock has
>> socket file and it passes f_flags as the flags param into the proto_ops
>> .connect.
>>
>> However, the sock created by sock_create_kern doesn't have a socket file,
>> and it passes the flags (like O_NONBLOCK) by using the flags param in
>> kernel_connect, which calls proto_ops .connect later.
>>
>> So to fix it, this patch defines a new proto_ops .connect for sctp,
>> sctp_inet_connect, which calls __sctp_connect() directly with this
>> flags param. After this, the sctp's proto .connect can be removed.
>>
>> Note that sctp_inet_connect doesn't need to do some checks that are not
>> needed for sctp, which makes thing better than with inet_dgram_connect.
>>
>> Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> Applied, thank you.
>
> I don't see a Fixes: tag, please give me some guidance me wrt. -stable.
The problem is there since the beginning, I think there's no need
to go to -stable.

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

end of thread, other threads:[~2018-05-23  6:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-20  8:39 [PATCH net] sctp: fix the issue that flags are ignored when using kernel_connect Xin Long
2018-05-21 11:46 ` Neil Horman
2018-05-21 15:52 ` Marcelo Ricardo Leitner
2018-05-22 12:38 ` Michal Kubecek
2018-05-22 17:40 ` David Miller
2018-05-23  6:55   ` Xin Long

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).