On Tue, Jun 20, 2017 at 12:41:47PM -0300, Marcelo Ricardo Leitner wrote: > Hi, > > On Tue, Jun 20, 2017 at 10:39:51AM -0400, Neil Horman wrote: > > On Tue, Jun 20, 2017 at 03:24:13PM +0200, Andreas Steinmetz wrote: > > > [please CC me, I'm not subscribed] > > > > > > It seems that if one does a getsockopt(SCTP_SOCKOPT_PEELOFF) a.k.a. > > > sctp_peeloff(), even if the socket descriptor from which the > > > association is to be peeled off has SOCK_CLOEXEC/SOCK_NONBLOCK set, > > > the peeled off socket descriptor doesn't have so. > > > > > > It would be advisable to either clone these flags or add an atomic > > > version of SCTP_SOCKOPT_PEELOFF (accept4() style). > > > > > > The missing SOCK_CLOEXEC requires unintentional additional locking > > > which typically is prone to errors and can slow down processing. > > > -- > > > Andreas Steinmetz SPAMmers use robotrap@xxxxxxxx > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > > > > > Try this patch, and confirm that it fixes the problem please? > > > > Thanks! > > Neil > > > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > > index f16c8d9..a95e3d6 100644 > > --- a/net/sctp/socket.c > > +++ b/net/sctp/socket.c > > @@ -4912,7 +4912,7 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp) > > return -EINVAL; > > > > /* Create a new socket. */ > > - err = sock_create(sk->sk_family, SOCK_SEQPACKET, IPPROTO_SCTP, &sock); > > + err = sock_create(sk->sk_family, sk->sk_type, IPPROTO_SCTP, &sock); > > I don't think this will work. Syscall socket() filters out these > flags and handle them separetly after calling sock_create() with just > the type, so I don't think this information is preserved in there. > > Interesting to note that according to man page dup() syscall also > doesn't duplicate these flags. One has to use dup3() instead in order to > be able to specify them. > > Maybe by extending sctp_peeloff_arg_t to have a flags attribute in > there, we can allow the application to specify it and feed into > get_unused_fd_flags() call in sctp_getsockopt_peeloff() instead, or even > just overload the sd, which is currently an output-only value, to > contain flags as the patch below. (We probably should add some sanity > checking in there, though) > Thinking about this some more, I'm a bit hesitant to change the sctp_peeloff_arg_t, since thats exposed to user space. Instead, what if we use the sd value in the peeloff arg to fetch the close_on_exec flag in the new fd? Something like this (untested) patch: diff --git a/net/sctp/socket.c b/net/sctp/socket.c index f16c8d9..6386ac4 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -4939,6 +4939,8 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval sctp_peeloff_arg_t peeloff; struct socket *newsock; struct file *newfile; + struct file *oldfile + unsigned flags = 0; int retval = 0; if (len < sizeof(sctp_peeloff_arg_t)) @@ -4951,8 +4953,17 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval if (retval < 0) goto out; + if (get_close_on_exec(peeloff.sd)) + flags |= O_CLOEXEC; + + oldfile = fget(peelof.sd); + if (oldfile) { + flags |= oldfile->f_flags & O_NONBLOCK; + fput(oldfile); + } + /* Map the socket to an unused fd that can be returned to the user. */ - retval = get_unused_fd_flags(0); + retval = get_unused_fd_flags(flags); if (retval < 0) { sock_release(newsock); goto out; -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
