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 <nhor...@tuxdriver.com>
> CC: Alexander Viro <v...@zeniv.linux.org.uk>
> CC: Vlad Yasevich <vyasev...@gmail.com>
> CC: "David S. Miller" <da...@davemloft.net>
> CC: Andreas Steinmetz <a...@domdv.de>
> CC: Marcelo Ricardo Leitner <marcelo.leit...@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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Reply via email to