* Re: [PATCH] sctp: Add peeloff-flags socket option
2017-06-29 18:13 [PATCH] sctp: Add peeloff-flags socket option Neil Horman
@ 2017-06-29 19:04 ` Marcelo Ricardo Leitner
2017-06-29 22:56 ` Al Viro
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-06-29 19:04 UTC (permalink / raw)
To: Neil Horman
Cc: linux-sctp, netdev, Alexander Viro, Vlad Yasevich,
David S. Miller, Andreas Steinmetz
On Thu, Jun 29, 2017 at 02:13:40PM -0400, Neil Horman wrote:
> Based on a request raised on the sctp devel list, there is a need to
> augment the sctp_peeloff operation while specifying the O_CLOEXEC and
> O_NONBLOCK flags (simmilar to the socket syscall). Since modifying the
> SCTP_SOCKOPT_PEELOFF socket option would break user space ABI for existing
> programs, this patch creates a new socket option
> SCTP_SOCKOPT_PEELOFF_FLAGS, which accepts a third flags parameter to
> allow atomic assignment of the socket descriptor flags.
>
> Tested successfully by myself and the requestor
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Alexander Viro <viro@zeniv.linux.org.uk>
> CC: Vlad Yasevich <vyasevich@gmail.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Andreas Steinmetz <ast@domdv.de>
> CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> fs/file.c | 1 +
> include/uapi/linux/sctp.h | 6 ++++
> net/sctp/socket.c | 92 ++++++++++++++++++++++++++++++++++++++---------
> 3 files changed, 83 insertions(+), 16 deletions(-)
>
> diff --git a/fs/file.c b/fs/file.c
> index 1c2972e..a4521da 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -807,6 +807,7 @@ void set_close_on_exec(unsigned int fd, int flag)
> __clear_close_on_exec(fd, fdt);
> spin_unlock(&files->file_lock);
> }
> +EXPORT_SYMBOL(set_close_on_exec);
>
> bool get_close_on_exec(unsigned int fd)
> {
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index ced9d8b..6217ff8 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -121,6 +121,7 @@ typedef __s32 sctp_assoc_t;
> #define SCTP_RESET_STREAMS 119
> #define SCTP_RESET_ASSOC 120
> #define SCTP_ADD_STREAMS 121
> +#define SCTP_SOCKOPT_PEELOFF_FLAGS 122
>
> /* PR-SCTP policies */
> #define SCTP_PR_SCTP_NONE 0x0000
> @@ -978,6 +979,11 @@ typedef struct {
> int sd;
> } sctp_peeloff_arg_t;
>
> +typedef struct {
> + sctp_peeloff_arg_t p_arg;
> + unsigned flags;
> +} sctp_peeloff_flags_arg_t;
> +
> /*
> * Peer Address Thresholds socket option
> */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 7b6e20e..7341053 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4933,20 +4933,12 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
> }
> EXPORT_SYMBOL(sctp_do_peeloff);
>
> -static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval, int __user *optlen)
> +static int sctp_getsockopt_peeloff_common(struct sock *sk, sctp_peeloff_arg_t *peeloff, struct file **newfile, unsigned flags)
> {
> - sctp_peeloff_arg_t peeloff;
> struct socket *newsock;
> - struct file *newfile;
> int retval = 0;
^^^ this initialization is not needed anymore
as the line after it is always assigning it.
>
> - if (len < sizeof(sctp_peeloff_arg_t))
> - return -EINVAL;
> - len = sizeof(sctp_peeloff_arg_t);
> - if (copy_from_user(&peeloff, optval, len))
> - return -EFAULT;
> -
> - retval = sctp_do_peeloff(sk, peeloff.associd, &newsock);
> + retval = sctp_do_peeloff(sk, peeloff->associd, &newsock);
> if (retval < 0)
> goto out;
>
> @@ -4957,25 +4949,90 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
> goto out;
> }
>
> - newfile = sock_alloc_file(newsock, 0, NULL);
> - if (IS_ERR(newfile)) {
> + *newfile = sock_alloc_file(newsock, 0, NULL);
> + if (IS_ERR(*newfile)) {
> put_unused_fd(retval);
> sock_release(newsock);
> - return PTR_ERR(newfile);
> + retval = PTR_ERR(*newfile);
> + *newfile = NULL;
> + return retval;
> }
>
> pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
> retval);
> +
> + peeloff->sd = retval;
> +
> + set_close_on_exec(peeloff->sd, !!(flags & SOCK_CLOEXEC));
You can avoid this call (and the export) if you pass flags &
SOCK_CLOEXEC to get_unused_fd_flags(), which currently uses flags=0.
> +
> + if (flags & SOCK_NONBLOCK)
> + (*newfile)->f_flags |= O_NONBLOCK;
and also to sock_alloc_file().
> +out:
> + return retval;
> +}
> +
> +static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval, int __user *optlen)
> +{
> + sctp_peeloff_arg_t peeloff;
> + struct file *newfile = NULL;
> + int retval = 0;
> +
> + if (len < sizeof(sctp_peeloff_arg_t))
> + return -EINVAL;
> + len = sizeof(sctp_peeloff_arg_t);
> + if (copy_from_user(&peeloff, optval, len))
> + return -EFAULT;
> +
> + retval = sctp_getsockopt_peeloff_common(sk, &peeloff, &newfile, 0);
> + if (retval < 0)
^^
> + goto out;
>
> /* Return the fd mapped to the new socket. */
> if (put_user(len, optlen)) {
> - fput(newfile);
> + if (newfile)
I don't think you need the if() here because for the file to not be
there, retval above will be negative and this part won't be called.
> + fput(newfile);
> put_unused_fd(retval);
> return -EFAULT;
> }
> - peeloff.sd = retval;
> +
> if (copy_to_user(optval, &peeloff, len)) {
> - fput(newfile);
> + if (newfile)
Same here and on sctp_getsockopt_peeloff_flags() below.
Marcelo
> + fput(newfile);
> + put_unused_fd(retval);
> + return -EFAULT;
> + }
> + fd_install(retval, newfile);
> +out:
> + return retval;
> +}
> +
> +static int sctp_getsockopt_peeloff_flags(struct sock *sk, int len, char __user *optval, int __user *optlen)
> +{
> + sctp_peeloff_flags_arg_t peeloff;
> + struct file *newfile = NULL;
> + int retval = 0;
> +
> + if (len < sizeof(sctp_peeloff_flags_arg_t))
> + return -EINVAL;
> + len = sizeof(sctp_peeloff_flags_arg_t);
> + if (copy_from_user(&peeloff, optval, len))
> + return -EFAULT;
> +
> + retval = sctp_getsockopt_peeloff_common(sk, &peeloff.p_arg, &newfile, peeloff.flags);
> + if (retval < 0)
> + goto out;
> +
> + /* Return the fd mapped to the new socket. */
> + if (put_user(len, optlen)) {
> + if (newfile)
> + fput(newfile);
> + put_unused_fd(retval);
> + return -EFAULT;
> + }
> +
> + if (copy_to_user(optval, &peeloff, len)) {
> + if (newfile)
> + fput(newfile);
> put_unused_fd(retval);
> return -EFAULT;
> }
> @@ -6758,6 +6815,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
> case SCTP_SOCKOPT_PEELOFF:
> retval = sctp_getsockopt_peeloff(sk, len, optval, optlen);
> break;
> + case SCTP_SOCKOPT_PEELOFF_FLAGS:
> + retval = sctp_getsockopt_peeloff_flags(sk, len, optval, optlen);
> + break;
> case SCTP_PEER_ADDR_PARAMS:
> retval = sctp_getsockopt_peer_addr_params(sk, len, optval,
> optlen);
> --
> 2.9.4
>
> --
> 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] 8+ messages in thread* Re: [PATCH] sctp: Add peeloff-flags socket option
2017-06-29 18:13 [PATCH] sctp: Add peeloff-flags socket option Neil Horman
2017-06-29 19:04 ` Marcelo Ricardo Leitner
@ 2017-06-29 22:56 ` Al Viro
2017-06-30 15:35 ` [PATCH v2] " Neil Horman
2017-06-30 17:32 ` [PATCH v3] " Neil Horman
3 siblings, 0 replies; 8+ messages in thread
From: Al Viro @ 2017-06-29 22:56 UTC (permalink / raw)
To: Neil Horman
Cc: linux-sctp, netdev, Vlad Yasevich, David S. Miller,
Andreas Steinmetz, Marcelo Ricardo Leitner
On Thu, Jun 29, 2017 at 02:13:40PM -0400, Neil Horman wrote:
> diff --git a/fs/file.c b/fs/file.c
> index 1c2972e..a4521da 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -807,6 +807,7 @@ void set_close_on_exec(unsigned int fd, int flag)
> __clear_close_on_exec(fd, fdt);
> spin_unlock(&files->file_lock);
> }
> +EXPORT_SYMBOL(set_close_on_exec);
Huh?
> + set_close_on_exec(peeloff->sd, !!(flags & SOCK_CLOEXEC));
Why hadn't you set it when that descriptor had been allocated?
That
retval = get_unused_fd_flags(0);
could just pass O_CLOEXEC and you'd get it set from the very
beginning...
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] sctp: Add peeloff-flags socket option
2017-06-29 18:13 [PATCH] sctp: Add peeloff-flags socket option Neil Horman
2017-06-29 19:04 ` Marcelo Ricardo Leitner
2017-06-29 22:56 ` Al Viro
@ 2017-06-30 15:35 ` Neil Horman
2017-06-30 16:29 ` Marcelo Ricardo Leitner
2017-06-30 17:32 ` [PATCH v3] " Neil Horman
3 siblings, 1 reply; 8+ messages in thread
From: Neil Horman @ 2017-06-30 15:35 UTC (permalink / raw)
To: linux-sctp
Cc: netdev, Neil Horman, Vlad Yasevich, David S. Miller,
Andreas Steinmetz, Marcelo Ricardo Leitner
Based on a request raised on the sctp devel list, there is a need to
augment the sctp_peeloff operation while specifying the O_CLOEXEC and
O_NONBLOCK flags (simmilar to the socket syscall). Since modifying the
SCTP_SOCKOPT_PEELOFF socket option would break user space ABI for existing
programs, this patch creates a new socket option
SCTP_SOCKOPT_PEELOFF_FLAGS, which accepts a third flags parameter to
allow atomic assignment of the socket descriptor flags.
Tested successfully by myself and the requestor
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Andreas Steinmetz <ast@domdv.de>
CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
Change notes:
v2)
* Per Marcellos comments, move to use of get_unused_fd_flags() and
clean up code to independently set CLOEXEC
* clean up a stray tab
---
include/uapi/linux/sctp.h | 6 ++++
net/sctp/socket.c | 84 ++++++++++++++++++++++++++++++++++++++---------
2 files changed, 75 insertions(+), 15 deletions(-)
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index ced9d8b..6217ff8 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -121,6 +121,7 @@ typedef __s32 sctp_assoc_t;
#define SCTP_RESET_STREAMS 119
#define SCTP_RESET_ASSOC 120
#define SCTP_ADD_STREAMS 121
+#define SCTP_SOCKOPT_PEELOFF_FLAGS 122
/* PR-SCTP policies */
#define SCTP_PR_SCTP_NONE 0x0000
@@ -978,6 +979,11 @@ typedef struct {
int sd;
} sctp_peeloff_arg_t;
+typedef struct {
+ sctp_peeloff_arg_t p_arg;
+ unsigned flags;
+} sctp_peeloff_flags_arg_t;
+
/*
* Peer Address Thresholds socket option
*/
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 7b6e20e..3cc5309 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4933,11 +4933,46 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
}
EXPORT_SYMBOL(sctp_do_peeloff);
+static int sctp_getsockopt_peeloff_common(struct sock *sk, sctp_peeloff_arg_t *peeloff, struct file **newfile, unsigned flags)
+{
+ struct socket *newsock;
+ int retval;
+
+ retval = sctp_do_peeloff(sk, peeloff->associd, &newsock);
+ if (retval < 0)
+ goto out;
+
+ /* Map the socket to an unused fd that can be returned to the user. */
+ retval = get_unused_fd_flags(flags & SOCK_CLOEXEC);
+ if (retval < 0) {
+ sock_release(newsock);
+ goto out;
+ }
+
+ *newfile = sock_alloc_file(newsock, 0, NULL);
+ if (IS_ERR(*newfile)) {
+ put_unused_fd(retval);
+ sock_release(newsock);
+ retval = PTR_ERR(*newfile);
+ *newfile = NULL;
+ return retval;
+ }
+
+ pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
+ retval);
+
+ peeloff->sd = retval;
+
+ if (flags & SOCK_NONBLOCK)
+ (*newfile)->f_flags |= O_NONBLOCK;
+out:
+ return retval;
+}
+
static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval, int __user *optlen)
{
sctp_peeloff_arg_t peeloff;
- struct socket *newsock;
- struct file *newfile;
+ struct file *newfile = NULL;
int retval = 0;
if (len < sizeof(sctp_peeloff_arg_t))
@@ -4946,26 +4981,42 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
if (copy_from_user(&peeloff, optval, len))
return -EFAULT;
- retval = sctp_do_peeloff(sk, peeloff.associd, &newsock);
+ retval = sctp_getsockopt_peeloff_common(sk, &peeloff, &newfile, 0);
if (retval < 0)
goto out;
- /* Map the socket to an unused fd that can be returned to the user. */
- retval = get_unused_fd_flags(0);
- if (retval < 0) {
- sock_release(newsock);
- goto out;
+ /* Return the fd mapped to the new socket. */
+ if (put_user(len, optlen)) {
+ fput(newfile);
+ put_unused_fd(retval);
+ return -EFAULT;
}
- newfile = sock_alloc_file(newsock, 0, NULL);
- if (IS_ERR(newfile)) {
+ if (copy_to_user(optval, &peeloff, len)) {
+ fput(newfile);
put_unused_fd(retval);
- sock_release(newsock);
- return PTR_ERR(newfile);
+ return -EFAULT;
}
+ fd_install(retval, newfile);
+out:
+ return retval;
+}
- pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
- retval);
+static int sctp_getsockopt_peeloff_flags(struct sock *sk, int len, char __user *optval, int __user *optlen)
+{
+ sctp_peeloff_flags_arg_t peeloff;
+ struct file *newfile = NULL;
+ int retval = 0;
+
+ if (len < sizeof(sctp_peeloff_flags_arg_t))
+ return -EINVAL;
+ len = sizeof(sctp_peeloff_flags_arg_t);
+ if (copy_from_user(&peeloff, optval, len))
+ return -EFAULT;
+
+ retval = sctp_getsockopt_peeloff_common(sk, &peeloff.p_arg, &newfile, peeloff.flags);
+ if (retval < 0)
+ goto out;
/* Return the fd mapped to the new socket. */
if (put_user(len, optlen)) {
@@ -4973,7 +5024,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
put_unused_fd(retval);
return -EFAULT;
}
- peeloff.sd = retval;
+
if (copy_to_user(optval, &peeloff, len)) {
fput(newfile);
put_unused_fd(retval);
@@ -6758,6 +6809,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
case SCTP_SOCKOPT_PEELOFF:
retval = sctp_getsockopt_peeloff(sk, len, optval, optlen);
break;
+ case SCTP_SOCKOPT_PEELOFF_FLAGS:
+ retval = sctp_getsockopt_peeloff_flags(sk, len, optval, optlen);
+ break;
case SCTP_PEER_ADDR_PARAMS:
retval = sctp_getsockopt_peer_addr_params(sk, len, optval,
optlen);
--
2.9.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] sctp: Add peeloff-flags socket option
2017-06-30 15:35 ` [PATCH v2] " Neil Horman
@ 2017-06-30 16:29 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-06-30 16:29 UTC (permalink / raw)
To: Neil Horman
Cc: linux-sctp, netdev, Vlad Yasevich, David S. Miller,
Andreas Steinmetz
On Fri, Jun 30, 2017 at 11:35:54AM -0400, Neil Horman wrote:
> Based on a request raised on the sctp devel list, there is a need to
> augment the sctp_peeloff operation while specifying the O_CLOEXEC and
> O_NONBLOCK flags (simmilar to the socket syscall). Since modifying the
> SCTP_SOCKOPT_PEELOFF socket option would break user space ABI for existing
> programs, this patch creates a new socket option
> SCTP_SOCKOPT_PEELOFF_FLAGS, which accepts a third flags parameter to
> allow atomic assignment of the socket descriptor flags.
>
> Tested successfully by myself and the requestor
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Vlad Yasevich <vyasevich@gmail.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Andreas Steinmetz <ast@domdv.de>
> CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
> ---
> Change notes:
>
> v2)
>
> * Per Marcellos comments, move to use of get_unused_fd_flags() and
> clean up code to independently set CLOEXEC
>
> * clean up a stray tab
> ---
> include/uapi/linux/sctp.h | 6 ++++
> net/sctp/socket.c | 84 ++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 75 insertions(+), 15 deletions(-)
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index ced9d8b..6217ff8 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -121,6 +121,7 @@ typedef __s32 sctp_assoc_t;
> #define SCTP_RESET_STREAMS 119
> #define SCTP_RESET_ASSOC 120
> #define SCTP_ADD_STREAMS 121
> +#define SCTP_SOCKOPT_PEELOFF_FLAGS 122
>
> /* PR-SCTP policies */
> #define SCTP_PR_SCTP_NONE 0x0000
> @@ -978,6 +979,11 @@ typedef struct {
> int sd;
> } sctp_peeloff_arg_t;
>
> +typedef struct {
> + sctp_peeloff_arg_t p_arg;
> + unsigned flags;
> +} sctp_peeloff_flags_arg_t;
> +
> /*
> * Peer Address Thresholds socket option
> */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 7b6e20e..3cc5309 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4933,11 +4933,46 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
> }
> EXPORT_SYMBOL(sctp_do_peeloff);
>
> +static int sctp_getsockopt_peeloff_common(struct sock *sk, sctp_peeloff_arg_t *peeloff, struct file **newfile, unsigned flags)
> +{
> + struct socket *newsock;
> + int retval;
> +
> + retval = sctp_do_peeloff(sk, peeloff->associd, &newsock);
> + if (retval < 0)
> + goto out;
> +
> + /* Map the socket to an unused fd that can be returned to the user. */
> + retval = get_unused_fd_flags(flags & SOCK_CLOEXEC);
> + if (retval < 0) {
> + sock_release(newsock);
> + goto out;
> + }
> +
> + *newfile = sock_alloc_file(newsock, 0, NULL);
> + if (IS_ERR(*newfile)) {
> + put_unused_fd(retval);
> + sock_release(newsock);
> + retval = PTR_ERR(*newfile);
> + *newfile = NULL;
> + return retval;
> + }
> +
> + pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
> + retval);
> +
> + peeloff->sd = retval;
> +
> + if (flags & SOCK_NONBLOCK)
> + (*newfile)->f_flags |= O_NONBLOCK;
> +out:
> + return retval;
> +}
> +
> static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval, int __user *optlen)
> {
> sctp_peeloff_arg_t peeloff;
> - struct socket *newsock;
> - struct file *newfile;
> + struct file *newfile = NULL;
> int retval = 0;
>
> if (len < sizeof(sctp_peeloff_arg_t))
> @@ -4946,26 +4981,42 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
> if (copy_from_user(&peeloff, optval, len))
> return -EFAULT;
>
> - retval = sctp_do_peeloff(sk, peeloff.associd, &newsock);
> + retval = sctp_getsockopt_peeloff_common(sk, &peeloff, &newfile, 0);
> if (retval < 0)
> goto out;
>
> - /* Map the socket to an unused fd that can be returned to the user. */
> - retval = get_unused_fd_flags(0);
> - if (retval < 0) {
> - sock_release(newsock);
> - goto out;
> + /* Return the fd mapped to the new socket. */
> + if (put_user(len, optlen)) {
> + fput(newfile);
> + put_unused_fd(retval);
> + return -EFAULT;
> }
>
> - newfile = sock_alloc_file(newsock, 0, NULL);
> - if (IS_ERR(newfile)) {
> + if (copy_to_user(optval, &peeloff, len)) {
> + fput(newfile);
> put_unused_fd(retval);
> - sock_release(newsock);
> - return PTR_ERR(newfile);
> + return -EFAULT;
> }
> + fd_install(retval, newfile);
> +out:
> + return retval;
> +}
>
> - pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
> - retval);
> +static int sctp_getsockopt_peeloff_flags(struct sock *sk, int len, char __user *optval, int __user *optlen)
> +{
> + sctp_peeloff_flags_arg_t peeloff;
> + struct file *newfile = NULL;
> + int retval = 0;
> +
> + if (len < sizeof(sctp_peeloff_flags_arg_t))
> + return -EINVAL;
> + len = sizeof(sctp_peeloff_flags_arg_t);
> + if (copy_from_user(&peeloff, optval, len))
> + return -EFAULT;
> +
> + retval = sctp_getsockopt_peeloff_common(sk, &peeloff.p_arg, &newfile, peeloff.flags);
That is a long line, would be nice to break it. It fits quite well if
broken before newfile parameter.
Marcelo
> + if (retval < 0)
> + goto out;
>
> /* Return the fd mapped to the new socket. */
> if (put_user(len, optlen)) {
> @@ -4973,7 +5024,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
> put_unused_fd(retval);
> return -EFAULT;
> }
> - peeloff.sd = retval;
> +
> if (copy_to_user(optval, &peeloff, len)) {
> fput(newfile);
> put_unused_fd(retval);
> @@ -6758,6 +6809,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
> case SCTP_SOCKOPT_PEELOFF:
> retval = sctp_getsockopt_peeloff(sk, len, optval, optlen);
> break;
> + case SCTP_SOCKOPT_PEELOFF_FLAGS:
> + retval = sctp_getsockopt_peeloff_flags(sk, len, optval, optlen);
> + break;
> case SCTP_PEER_ADDR_PARAMS:
> retval = sctp_getsockopt_peer_addr_params(sk, len, optval,
> optlen);
> --
> 2.9.4
>
> --
> 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] 8+ messages in thread
* [PATCH v3] sctp: Add peeloff-flags socket option
2017-06-29 18:13 [PATCH] sctp: Add peeloff-flags socket option Neil Horman
` (2 preceding siblings ...)
2017-06-30 15:35 ` [PATCH v2] " Neil Horman
@ 2017-06-30 17:32 ` Neil Horman
2017-07-01 17:29 ` Marcelo Ricardo Leitner
2017-07-01 22:26 ` David Miller
3 siblings, 2 replies; 8+ messages in thread
From: Neil Horman @ 2017-06-30 17:32 UTC (permalink / raw)
To: linux-sctp
Cc: netdev, Neil Horman, Vlad Yasevich, David S. Miller,
Andreas Steinmetz, Marcelo Ricardo Leitner
Based on a request raised on the sctp devel list, there is a need to
augment the sctp_peeloff operation while specifying the O_CLOEXEC and
O_NONBLOCK flags (simmilar to the socket syscall). Since modifying the
SCTP_SOCKOPT_PEELOFF socket option would break user space ABI for existing
programs, this patch creates a new socket option
SCTP_SOCKOPT_PEELOFF_FLAGS, which accepts a third flags parameter to
allow atomic assignment of the socket descriptor flags.
Tested successfully by myself and the requestor
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Andreas Steinmetz <ast@domdv.de>
CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
Change notes:
v2)
* Per Marcellos comments, move to use of get_unused_fd_flags() and
clean up code to independently set CLOEXEC
* clean up a stray tab
v3)
* Clean up some long lines
---
include/uapi/linux/sctp.h | 6 ++++
net/sctp/socket.c | 87 +++++++++++++++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index ced9d8b..6217ff8 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -121,6 +121,7 @@ typedef __s32 sctp_assoc_t;
#define SCTP_RESET_STREAMS 119
#define SCTP_RESET_ASSOC 120
#define SCTP_ADD_STREAMS 121
+#define SCTP_SOCKOPT_PEELOFF_FLAGS 122
/* PR-SCTP policies */
#define SCTP_PR_SCTP_NONE 0x0000
@@ -978,6 +979,11 @@ typedef struct {
int sd;
} sctp_peeloff_arg_t;
+typedef struct {
+ sctp_peeloff_arg_t p_arg;
+ unsigned flags;
+} sctp_peeloff_flags_arg_t;
+
/*
* Peer Address Thresholds socket option
*/
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 7b6e20e..72de049 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4933,11 +4933,47 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
}
EXPORT_SYMBOL(sctp_do_peeloff);
+static int sctp_getsockopt_peeloff_common(struct sock *sk, sctp_peeloff_arg_t *peeloff,
+ struct file **newfile, unsigned flags)
+{
+ struct socket *newsock;
+ int retval;
+
+ retval = sctp_do_peeloff(sk, peeloff->associd, &newsock);
+ if (retval < 0)
+ goto out;
+
+ /* Map the socket to an unused fd that can be returned to the user. */
+ retval = get_unused_fd_flags(flags & SOCK_CLOEXEC);
+ if (retval < 0) {
+ sock_release(newsock);
+ goto out;
+ }
+
+ *newfile = sock_alloc_file(newsock, 0, NULL);
+ if (IS_ERR(*newfile)) {
+ put_unused_fd(retval);
+ sock_release(newsock);
+ retval = PTR_ERR(*newfile);
+ *newfile = NULL;
+ return retval;
+ }
+
+ pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
+ retval);
+
+ peeloff->sd = retval;
+
+ if (flags & SOCK_NONBLOCK)
+ (*newfile)->f_flags |= O_NONBLOCK;
+out:
+ return retval;
+}
+
static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval, int __user *optlen)
{
sctp_peeloff_arg_t peeloff;
- struct socket *newsock;
- struct file *newfile;
+ struct file *newfile = NULL;
int retval = 0;
if (len < sizeof(sctp_peeloff_arg_t))
@@ -4946,26 +4982,44 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
if (copy_from_user(&peeloff, optval, len))
return -EFAULT;
- retval = sctp_do_peeloff(sk, peeloff.associd, &newsock);
+ retval = sctp_getsockopt_peeloff_common(sk, &peeloff, &newfile, 0);
if (retval < 0)
goto out;
- /* Map the socket to an unused fd that can be returned to the user. */
- retval = get_unused_fd_flags(0);
- if (retval < 0) {
- sock_release(newsock);
- goto out;
+ /* Return the fd mapped to the new socket. */
+ if (put_user(len, optlen)) {
+ fput(newfile);
+ put_unused_fd(retval);
+ return -EFAULT;
}
- newfile = sock_alloc_file(newsock, 0, NULL);
- if (IS_ERR(newfile)) {
+ if (copy_to_user(optval, &peeloff, len)) {
+ fput(newfile);
put_unused_fd(retval);
- sock_release(newsock);
- return PTR_ERR(newfile);
+ return -EFAULT;
}
+ fd_install(retval, newfile);
+out:
+ return retval;
+}
- pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
- retval);
+static int sctp_getsockopt_peeloff_flags(struct sock *sk, int len,
+ char __user *optval, int __user *optlen)
+{
+ sctp_peeloff_flags_arg_t peeloff;
+ struct file *newfile = NULL;
+ int retval = 0;
+
+ if (len < sizeof(sctp_peeloff_flags_arg_t))
+ return -EINVAL;
+ len = sizeof(sctp_peeloff_flags_arg_t);
+ if (copy_from_user(&peeloff, optval, len))
+ return -EFAULT;
+
+ retval = sctp_getsockopt_peeloff_common(sk, &peeloff.p_arg,
+ &newfile, peeloff.flags);
+ if (retval < 0)
+ goto out;
/* Return the fd mapped to the new socket. */
if (put_user(len, optlen)) {
@@ -4973,7 +5027,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
put_unused_fd(retval);
return -EFAULT;
}
- peeloff.sd = retval;
+
if (copy_to_user(optval, &peeloff, len)) {
fput(newfile);
put_unused_fd(retval);
@@ -6758,6 +6812,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
case SCTP_SOCKOPT_PEELOFF:
retval = sctp_getsockopt_peeloff(sk, len, optval, optlen);
break;
+ case SCTP_SOCKOPT_PEELOFF_FLAGS:
+ retval = sctp_getsockopt_peeloff_flags(sk, len, optval, optlen);
+ break;
case SCTP_PEER_ADDR_PARAMS:
retval = sctp_getsockopt_peer_addr_params(sk, len, optval,
optlen);
--
2.9.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] sctp: Add peeloff-flags socket option
2017-06-30 17:32 ` [PATCH v3] " Neil Horman
@ 2017-07-01 17:29 ` Marcelo Ricardo Leitner
2017-07-01 22:26 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-07-01 17:29 UTC (permalink / raw)
To: Neil Horman
Cc: linux-sctp, netdev, Vlad Yasevich, David S. Miller,
Andreas Steinmetz
On Fri, Jun 30, 2017 at 01:32:57PM -0400, Neil Horman wrote:
> Based on a request raised on the sctp devel list, there is a need to
> augment the sctp_peeloff operation while specifying the O_CLOEXEC and
> O_NONBLOCK flags (simmilar to the socket syscall). Since modifying the
> SCTP_SOCKOPT_PEELOFF socket option would break user space ABI for existing
> programs, this patch creates a new socket option
> SCTP_SOCKOPT_PEELOFF_FLAGS, which accepts a third flags parameter to
> allow atomic assignment of the socket descriptor flags.
>
> Tested successfully by myself and the requestor
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Vlad Yasevich <vyasevich@gmail.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Andreas Steinmetz <ast@domdv.de>
> CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Thanks!
>
> ---
> Change notes:
>
> v2)
>
> * Per Marcellos comments, move to use of get_unused_fd_flags() and
> clean up code to independently set CLOEXEC
>
> * clean up a stray tab
>
> v3)
>
> * Clean up some long lines
> ---
> include/uapi/linux/sctp.h | 6 ++++
> net/sctp/socket.c | 87 +++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 78 insertions(+), 15 deletions(-)
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index ced9d8b..6217ff8 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -121,6 +121,7 @@ typedef __s32 sctp_assoc_t;
> #define SCTP_RESET_STREAMS 119
> #define SCTP_RESET_ASSOC 120
> #define SCTP_ADD_STREAMS 121
> +#define SCTP_SOCKOPT_PEELOFF_FLAGS 122
>
> /* PR-SCTP policies */
> #define SCTP_PR_SCTP_NONE 0x0000
> @@ -978,6 +979,11 @@ typedef struct {
> int sd;
> } sctp_peeloff_arg_t;
>
> +typedef struct {
> + sctp_peeloff_arg_t p_arg;
> + unsigned flags;
> +} sctp_peeloff_flags_arg_t;
> +
> /*
> * Peer Address Thresholds socket option
> */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 7b6e20e..72de049 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4933,11 +4933,47 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
> }
> EXPORT_SYMBOL(sctp_do_peeloff);
>
> +static int sctp_getsockopt_peeloff_common(struct sock *sk, sctp_peeloff_arg_t *peeloff,
> + struct file **newfile, unsigned flags)
> +{
> + struct socket *newsock;
> + int retval;
> +
> + retval = sctp_do_peeloff(sk, peeloff->associd, &newsock);
> + if (retval < 0)
> + goto out;
> +
> + /* Map the socket to an unused fd that can be returned to the user. */
> + retval = get_unused_fd_flags(flags & SOCK_CLOEXEC);
> + if (retval < 0) {
> + sock_release(newsock);
> + goto out;
> + }
> +
> + *newfile = sock_alloc_file(newsock, 0, NULL);
> + if (IS_ERR(*newfile)) {
> + put_unused_fd(retval);
> + sock_release(newsock);
> + retval = PTR_ERR(*newfile);
> + *newfile = NULL;
> + return retval;
> + }
> +
> + pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
> + retval);
> +
> + peeloff->sd = retval;
> +
> + if (flags & SOCK_NONBLOCK)
> + (*newfile)->f_flags |= O_NONBLOCK;
> +out:
> + return retval;
> +}
> +
> static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval, int __user *optlen)
> {
> sctp_peeloff_arg_t peeloff;
> - struct socket *newsock;
> - struct file *newfile;
> + struct file *newfile = NULL;
> int retval = 0;
>
> if (len < sizeof(sctp_peeloff_arg_t))
> @@ -4946,26 +4982,44 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
> if (copy_from_user(&peeloff, optval, len))
> return -EFAULT;
>
> - retval = sctp_do_peeloff(sk, peeloff.associd, &newsock);
> + retval = sctp_getsockopt_peeloff_common(sk, &peeloff, &newfile, 0);
> if (retval < 0)
> goto out;
>
> - /* Map the socket to an unused fd that can be returned to the user. */
> - retval = get_unused_fd_flags(0);
> - if (retval < 0) {
> - sock_release(newsock);
> - goto out;
> + /* Return the fd mapped to the new socket. */
> + if (put_user(len, optlen)) {
> + fput(newfile);
> + put_unused_fd(retval);
> + return -EFAULT;
> }
>
> - newfile = sock_alloc_file(newsock, 0, NULL);
> - if (IS_ERR(newfile)) {
> + if (copy_to_user(optval, &peeloff, len)) {
> + fput(newfile);
> put_unused_fd(retval);
> - sock_release(newsock);
> - return PTR_ERR(newfile);
> + return -EFAULT;
> }
> + fd_install(retval, newfile);
> +out:
> + return retval;
> +}
>
> - pr_debug("%s: sk:%p, newsk:%p, sd:%d\n", __func__, sk, newsock->sk,
> - retval);
> +static int sctp_getsockopt_peeloff_flags(struct sock *sk, int len,
> + char __user *optval, int __user *optlen)
> +{
> + sctp_peeloff_flags_arg_t peeloff;
> + struct file *newfile = NULL;
> + int retval = 0;
> +
> + if (len < sizeof(sctp_peeloff_flags_arg_t))
> + return -EINVAL;
> + len = sizeof(sctp_peeloff_flags_arg_t);
> + if (copy_from_user(&peeloff, optval, len))
> + return -EFAULT;
> +
> + retval = sctp_getsockopt_peeloff_common(sk, &peeloff.p_arg,
> + &newfile, peeloff.flags);
> + if (retval < 0)
> + goto out;
>
> /* Return the fd mapped to the new socket. */
> if (put_user(len, optlen)) {
> @@ -4973,7 +5027,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
> put_unused_fd(retval);
> return -EFAULT;
> }
> - peeloff.sd = retval;
> +
> if (copy_to_user(optval, &peeloff, len)) {
> fput(newfile);
> put_unused_fd(retval);
> @@ -6758,6 +6812,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
> case SCTP_SOCKOPT_PEELOFF:
> retval = sctp_getsockopt_peeloff(sk, len, optval, optlen);
> break;
> + case SCTP_SOCKOPT_PEELOFF_FLAGS:
> + retval = sctp_getsockopt_peeloff_flags(sk, len, optval, optlen);
> + break;
> case SCTP_PEER_ADDR_PARAMS:
> retval = sctp_getsockopt_peer_addr_params(sk, len, optval,
> optlen);
> --
> 2.9.4
>
> --
> 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] 8+ messages in thread* Re: [PATCH v3] sctp: Add peeloff-flags socket option
2017-06-30 17:32 ` [PATCH v3] " Neil Horman
2017-07-01 17:29 ` Marcelo Ricardo Leitner
@ 2017-07-01 22:26 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2017-07-01 22:26 UTC (permalink / raw)
To: nhorman; +Cc: linux-sctp, netdev, vyasevich, ast, marcelo.leitner
From: Neil Horman <nhorman@tuxdriver.com>
Date: Fri, 30 Jun 2017 13:32:57 -0400
> Based on a request raised on the sctp devel list, there is a need to
> augment the sctp_peeloff operation while specifying the O_CLOEXEC and
> O_NONBLOCK flags (simmilar to the socket syscall). Since modifying the
> SCTP_SOCKOPT_PEELOFF socket option would break user space ABI for existing
> programs, this patch creates a new socket option
> SCTP_SOCKOPT_PEELOFF_FLAGS, which accepts a third flags parameter to
> allow atomic assignment of the socket descriptor flags.
>
> Tested successfully by myself and the requestor
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied to net-next, thanks Neil.
^ permalink raw reply [flat|nested] 8+ messages in thread