Re: linux-next: manual merge of the selinux tree with the net-next tree

[Paul Moore]

On Wed, Mar 7, 2018 at 9:07 PM, Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
> Hi all,
>
> On Mon, 5 Mar 2018 12:40:54 +1100 Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
>>
>> Today's linux-next merge of the selinux tree got a conflict in:
>>
>>   net/sctp/socket.c
>>
>> between several refactoring commits from the net-next tree and commit:
>>
>>   2277c7cd75e3 ("sctp: Add LSM hooks")
>>
>> from the selinux tree.
>>
>> I fixed it up (I think - see below) and can carry the fix as
>> necessary. This is now fixed as far as linux-next is concerned, but any
>> non trivial conflicts should be mentioned to your upstream maintainer
>> when your tree is submitted for merging.  You may also want to consider
>> cooperating with the maintainer of the conflicting tree to minimise any
>> particularly complex conflicts.
>>
>> --
>> Cheers,
>> Stephen Rothwell
>
> The resolution now looks like below (there were more changes to this
> file in the net-next tree).  It will keep changing every time this file
> is touched :-(

Xin Long, does this still look okay to you?

> diff --cc net/sctp/socket.c
> index 7d3476a4860d,73b34a6b5b09..000000000000
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@@ -1606,200 -1622,308 +1622,216 @@@ static int sctp_error(struct sock *sk,
>   static int sctp_msghdr_parse(const struct msghdr *msg,
>                              struct sctp_cmsgs *cmsgs);
>
>  -static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
>  +static int sctp_sendmsg_parse(struct sock *sk, struct sctp_cmsgs *cmsgs,
>  +                            struct sctp_sndrcvinfo *srinfo,
>  +                            const struct msghdr *msg, size_t msg_len)
>   {
>  -      struct net *net = sock_net(sk);
>  -      struct sctp_sock *sp;
>  -      struct sctp_endpoint *ep;
>  -      struct sctp_association *new_asoc = NULL, *asoc = NULL;
>  -      struct sctp_transport *transport, *chunk_tp;
>  -      struct sctp_chunk *chunk;
>  -      union sctp_addr to;
>  -      struct sctp_af *af;
>  -      struct sockaddr *msg_name = NULL;
>  -      struct sctp_sndrcvinfo default_sinfo;
>  -      struct sctp_sndrcvinfo *sinfo;
>  -      struct sctp_initmsg *sinit;
>  -      sctp_assoc_t associd = 0;
>  -      struct sctp_cmsgs cmsgs = { NULL };
>  -      enum sctp_scope scope;
>  -      bool fill_sinfo_ttl = false, wait_connect = false;
>  -      struct sctp_datamsg *datamsg;
>  -      int msg_flags = msg->msg_flags;
>  -      __u16 sinfo_flags = 0;
>  -      long timeo;
>  +      __u16 sflags;
>         int err;
>
>  -      err = 0;
>  -      sp = sctp_sk(sk);
>  -      ep = sp->ep;
>  +      if (sctp_sstate(sk, LISTENING) && sctp_style(sk, TCP))
>  +              return -EPIPE;
>
>  -      pr_debug("%s: sk:%p, msg:%p, msg_len:%zu ep:%p\n", __func__, sk,
>  -               msg, msg_len, ep);
>  -
>  -      /* We cannot send a message over a TCP-style listening socket. */
>  -      if (sctp_style(sk, TCP) && sctp_sstate(sk, LISTENING)) {
>  -              err = -EPIPE;
>  -              goto out_nounlock;
>  -      }
>  +      if (msg_len > sk->sk_sndbuf)
>  +              return -EMSGSIZE;
>
>  -      /* Parse out the SCTP CMSGs.  */
>  -      err = sctp_msghdr_parse(msg, &cmsgs);
>  +      memset(cmsgs, 0, sizeof(*cmsgs));
>  +      err = sctp_msghdr_parse(msg, cmsgs);
>         if (err) {
>                 pr_debug("%s: msghdr parse err:%x\n", __func__, err);
>  -              goto out_nounlock;
>  +              return err;
>         }
>
>  -      /* Fetch the destination address for this packet.  This
>  -       * address only selects the association--it is not necessarily
>  -       * the address we will send to.
>  -       * For a peeled-off socket, msg_name is ignored.
>  -       */
>  -      if (!sctp_style(sk, UDP_HIGH_BANDWIDTH) && msg->msg_name) {
>  -              int msg_namelen = msg->msg_namelen;
>  +      memset(srinfo, 0, sizeof(*srinfo));
>  +      if (cmsgs->srinfo) {
>  +              srinfo->sinfo_stream = cmsgs->srinfo->sinfo_stream;
>  +              srinfo->sinfo_flags = cmsgs->srinfo->sinfo_flags;
>  +              srinfo->sinfo_ppid = cmsgs->srinfo->sinfo_ppid;
>  +              srinfo->sinfo_context = cmsgs->srinfo->sinfo_context;
>  +              srinfo->sinfo_assoc_id = cmsgs->srinfo->sinfo_assoc_id;
>  +              srinfo->sinfo_timetolive = cmsgs->srinfo->sinfo_timetolive;
>  +      }
>
>  -              err = sctp_verify_addr(sk, (union sctp_addr *)msg->msg_name,
>  -                                     msg_namelen);
>  -              if (err)
>  -                      return err;
>  +      if (cmsgs->sinfo) {
>  +              srinfo->sinfo_stream = cmsgs->sinfo->snd_sid;
>  +              srinfo->sinfo_flags = cmsgs->sinfo->snd_flags;
>  +              srinfo->sinfo_ppid = cmsgs->sinfo->snd_ppid;
>  +              srinfo->sinfo_context = cmsgs->sinfo->snd_context;
>  +              srinfo->sinfo_assoc_id = cmsgs->sinfo->snd_assoc_id;
>  +      }
>
>  -              if (msg_namelen > sizeof(to))
>  -                      msg_namelen = sizeof(to);
>  -              memcpy(&to, msg->msg_name, msg_namelen);
>  -              msg_name = msg->msg_name;
>  +      if (cmsgs->prinfo) {
>  +              srinfo->sinfo_timetolive = cmsgs->prinfo->pr_value;
>  +              SCTP_PR_SET_POLICY(srinfo->sinfo_flags,
>  +                                 cmsgs->prinfo->pr_policy);
>         }
>
>  -      sinit = cmsgs.init;
>  -      if (cmsgs.sinfo != NULL) {
>  -              memset(&default_sinfo, 0, sizeof(default_sinfo));
>  -              default_sinfo.sinfo_stream = cmsgs.sinfo->snd_sid;
>  -              default_sinfo.sinfo_flags = cmsgs.sinfo->snd_flags;
>  -              default_sinfo.sinfo_ppid = cmsgs.sinfo->snd_ppid;
>  -              default_sinfo.sinfo_context = cmsgs.sinfo->snd_context;
>  -              default_sinfo.sinfo_assoc_id = cmsgs.sinfo->snd_assoc_id;
>  +      sflags = srinfo->sinfo_flags;
>  +      if (!sflags && msg_len)
>  +              return 0;
>
>  -              sinfo = &default_sinfo;
>  -              fill_sinfo_ttl = true;
>  -      } else {
>  -              sinfo = cmsgs.srinfo;
>  -      }
>  -      /* Did the user specify SNDINFO/SNDRCVINFO? */
>  -      if (sinfo) {
>  -              sinfo_flags = sinfo->sinfo_flags;
>  -              associd = sinfo->sinfo_assoc_id;
>  -      }
>  +      if (sctp_style(sk, TCP) && (sflags & (SCTP_EOF | SCTP_ABORT)))
>  +              return -EINVAL;
>
>  -      pr_debug("%s: msg_len:%zu, sinfo_flags:0x%x\n", __func__,
>  -               msg_len, sinfo_flags);
>  +      if (((sflags & SCTP_EOF) && msg_len > 0) ||
>  +          (!(sflags & (SCTP_EOF | SCTP_ABORT)) && msg_len == 0))
>  +              return -EINVAL;
>
>  -      /* SCTP_EOF or SCTP_ABORT cannot be set on a TCP-style socket. */
>  -      if (sctp_style(sk, TCP) && (sinfo_flags & (SCTP_EOF | SCTP_ABORT))) {
>  -              err = -EINVAL;
>  -              goto out_nounlock;
>  -      }
>  +      if ((sflags & SCTP_ADDR_OVER) && !msg->msg_name)
>  +              return -EINVAL;
>
>  -      /* If SCTP_EOF is set, no data can be sent. Disallow sending zero
>  -       * length messages when SCTP_EOF|SCTP_ABORT is not set.
>  -       * If SCTP_ABORT is set, the message length could be non zero with
>  -       * the msg_iov set to the user abort reason.
>  -       */
>  -      if (((sinfo_flags & SCTP_EOF) && (msg_len > 0)) ||
>  -          (!(sinfo_flags & (SCTP_EOF|SCTP_ABORT)) && (msg_len == 0))) {
>  -              err = -EINVAL;
>  -              goto out_nounlock;
>  -      }
>  +      return 0;
>  +}
>
>  -      /* If SCTP_ADDR_OVER is set, there must be an address
>  -       * specified in msg_name.
>  -       */
>  -      if ((sinfo_flags & SCTP_ADDR_OVER) && (!msg->msg_name)) {
>  -              err = -EINVAL;
>  -              goto out_nounlock;
>  -      }
>  +static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
>  +                               struct sctp_cmsgs *cmsgs,
>  +                               union sctp_addr *daddr,
>  +                               struct sctp_transport **tp)
>  +{
>  +      struct sctp_endpoint *ep = sctp_sk(sk)->ep;
>  +      struct net *net = sock_net(sk);
>  +      struct sctp_association *asoc;
>  +      enum sctp_scope scope;
>  +      struct cmsghdr *cmsg;
> ++      struct sctp_af *af;
>  +      int err = -EINVAL;
>
>  -      transport = NULL;
>  +      *tp = NULL;
>
>  -      pr_debug("%s: about to look up association\n", __func__);
>  +      if (sflags & (SCTP_EOF | SCTP_ABORT))
>  +              return -EINVAL;
>
>  -      lock_sock(sk);
>  +      if (sctp_style(sk, TCP) && (sctp_sstate(sk, ESTABLISHED) ||
>  +                                  sctp_sstate(sk, CLOSING)))
>  +              return -EADDRNOTAVAIL;
>
>  -      /* If a msg_name has been specified, assume this is to be used.  */
>  -      if (msg_name) {
>  -              /* Look for a matching association on the endpoint. */
>  -              asoc = sctp_endpoint_lookup_assoc(ep, &to, &transport);
>  +      if (sctp_endpoint_is_peeled_off(ep, daddr))
>  +              return -EADDRNOTAVAIL;
>
>  -              /* If we could not find a matching association on the
>  -               * endpoint, make sure that it is not a TCP-style
>  -               * socket that already has an association or there is
>  -               * no peeled-off association on another socket.
>  -               */
>  -              if (!asoc &&
>  -                  ((sctp_style(sk, TCP) &&
>  -                    (sctp_sstate(sk, ESTABLISHED) ||
>  -                     sctp_sstate(sk, CLOSING))) ||
>  -                   sctp_endpoint_is_peeled_off(ep, &to))) {
>  -                      err = -EADDRNOTAVAIL;
>  -                      goto out_unlock;
>  -              }
>  +      if (!ep->base.bind_addr.port) {
>  +              if (sctp_autobind(sk))
>  +                      return -EAGAIN;
>         } else {
>  -              asoc = sctp_id2assoc(sk, associd);
>  -              if (!asoc) {
>  -                      err = -EPIPE;
>  -                      goto out_unlock;
>  -              }
>  +              if (ep->base.bind_addr.port < inet_prot_sock(net) &&
>  +                  !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE))
>  +                      return -EACCES;
>         }
>
>  -      if (asoc) {
>  -              pr_debug("%s: just looked up association:%p\n", __func__, asoc);
>  +      scope = sctp_scope(daddr);
>
>  -              /* We cannot send a message on a TCP-style SCTP_SS_ESTABLISHED
>  -               * socket that has an association in CLOSED state. This can
>  -               * happen when an accepted socket has an association that is
>  -               * already CLOSED.
>  -               */
>  -              if (sctp_state(asoc, CLOSED) && sctp_style(sk, TCP)) {
>  -                      err = -EPIPE;
>  -                      goto out_unlock;
>  -              }
> ++      /* Label connection socket for first association 1-to-many
> ++       * style for client sequence socket()->sendmsg(). This
> ++       * needs to be done before sctp_assoc_add_peer() as that will
> ++       * set up the initial packet that needs to account for any
> ++       * security ip options (CIPSO/CALIPSO) added to the packet.
> ++       */
> ++      af = sctp_get_af_specific(daddr->sa.sa_family);
> ++      if (!af)
> ++              return -EINVAL;
> ++      err = security_sctp_bind_connect(sk, SCTP_SENDMSG_CONNECT,
> ++                                       (struct sockaddr *)daddr,
> ++                                       af->sockaddr_len);
> ++      if (err < 0)
> ++              return err;
> +
>  -              if (sinfo_flags & SCTP_EOF) {
>  -                      pr_debug("%s: shutting down association:%p\n",
>  -                               __func__, asoc);
>  +      asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
>  +      if (!asoc)
>  +              return -ENOMEM;
>
>  -                      sctp_primitive_SHUTDOWN(net, asoc, NULL);
>  -                      err = 0;
>  -                      goto out_unlock;
>  -              }
>  -              if (sinfo_flags & SCTP_ABORT) {
>  +      if (sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL) < 0) {
>  +              err = -ENOMEM;
>  +              goto free;
>  +      }
>
>  -                      chunk = sctp_make_abort_user(asoc, msg, msg_len);
>  -                      if (!chunk) {
>  -                              err = -ENOMEM;
>  -                              goto out_unlock;
>  -                      }
>  +      if (cmsgs->init) {
>  +              struct sctp_initmsg *init = cmsgs->init;
>
>  -                      pr_debug("%s: aborting association:%p\n",
>  -                               __func__, asoc);
>  +              if (init->sinit_num_ostreams) {
>  +                      __u16 outcnt = init->sinit_num_ostreams;
>
>  -                      sctp_primitive_ABORT(net, asoc, chunk);
>  -                      err = 0;
>  -                      goto out_unlock;
>  +                      asoc->c.sinit_num_ostreams = outcnt;
>  +                      /* outcnt has been changed, need to re-init stream */
>  +                      err = sctp_stream_init(&asoc->stream, outcnt, 0,
>  +                                             GFP_KERNEL);
>  +                      if (err)
>  +                              goto free;
>                 }
>  +
>  +              if (init->sinit_max_instreams)
>  +                      asoc->c.sinit_max_instreams = init->sinit_max_instreams;
>  +
>  +              if (init->sinit_max_attempts)
>  +                      asoc->max_init_attempts = init->sinit_max_attempts;
>  +
>  +              if (init->sinit_max_init_timeo)
>  +                      asoc->max_init_timeo =
>  +                              msecs_to_jiffies(init->sinit_max_init_timeo);
>         }
>
>  -      /* Do we need to create the association?  */
>  -      if (!asoc) {
>  -              pr_debug("%s: there is no association yet\n", __func__);
>  +      *tp = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL, SCTP_UNKNOWN);
>  +      if (!*tp) {
>  +              err = -ENOMEM;
>  +              goto free;
>  +      }
>
>  -              if (sinfo_flags & (SCTP_EOF | SCTP_ABORT)) {
>  -                      err = -EINVAL;
>  -                      goto out_unlock;
>  -              }
>  +      if (!cmsgs->addrs_msg)
>  +              return 0;
>
>  -              /* Check for invalid stream against the stream counts,
>  -               * either the default or the user specified stream counts.
>  -               */
>  -              if (sinfo) {
>  -                      if (!sinit || !sinit->sinit_num_ostreams) {
>  -                              /* Check against the defaults. */
>  -                              if (sinfo->sinfo_stream >=
>  -                                  sp->initmsg.sinit_num_ostreams) {
>  -                                      err = -EINVAL;
>  -                                      goto out_unlock;
>  -                              }
>  -                      } else {
>  -                              /* Check against the requested.  */
>  -                              if (sinfo->sinfo_stream >=
>  -                                  sinit->sinit_num_ostreams) {
>  -                                      err = -EINVAL;
>  -                                      goto out_unlock;
>  -                              }
>  -                      }
>  -              }
>  +      /* sendv addr list parse */
>  +      for_each_cmsghdr(cmsg, cmsgs->addrs_msg) {
>  +              struct sctp_transport *transport;
>  +              struct sctp_association *old;
>  +              union sctp_addr _daddr;
>  +              int dlen;
>
>  -              /*
>  -               * API 3.1.2 bind() - UDP Style Syntax
>  -               * If a bind() or sctp_bindx() is not called prior to a
>  -               * sendmsg() call that initiates a new association, the
>  -               * system picks an ephemeral port and will choose an address
>  -               * set equivalent to binding with a wildcard address.
>  -               */
>  -              if (!ep->base.bind_addr.port) {
>  -                      if (sctp_autobind(sk)) {
>  -                              err = -EAGAIN;
>  -                              goto out_unlock;
>  -                      }
>  -              } else {
>  -                      /*
>  -                       * If an unprivileged user inherits a one-to-many
>  -                       * style socket with open associations on a privileged
>  -                       * port, it MAY be permitted to accept new associations,
>  -                       * but it SHOULD NOT be permitted to open new
>  -                       * associations.
>  -                       */
>  -                      if (ep->base.bind_addr.port < inet_prot_sock(net) &&
>  -                          !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE)) {
>  -                              err = -EACCES;
>  -                              goto out_unlock;
>  -                      }
>  -              }
>  +              if (cmsg->cmsg_level != IPPROTO_SCTP ||
>  +                  (cmsg->cmsg_type != SCTP_DSTADDRV4 &&
>  +                   cmsg->cmsg_type != SCTP_DSTADDRV6))
>  +                      continue;
>
>  -              scope = sctp_scope(&to);
>  +              daddr = &_daddr;
>  +              memset(daddr, 0, sizeof(*daddr));
>  +              dlen = cmsg->cmsg_len - sizeof(struct cmsghdr);
>  +              if (cmsg->cmsg_type == SCTP_DSTADDRV4) {
>  +                      if (dlen < sizeof(struct in_addr))
>  +                              goto free;
>  +
>  +                      dlen = sizeof(struct in_addr);
>  +                      daddr->v4.sin_family = AF_INET;
>  +                      daddr->v4.sin_port = htons(asoc->peer.port);
>  +                      memcpy(&daddr->v4.sin_addr, CMSG_DATA(cmsg), dlen);
>  +              } else {
>  +                      if (dlen < sizeof(struct in6_addr))
>  +                              goto free;
>
>  -              /* Label connection socket for first association 1-to-many
>  -               * style for client sequence socket()->sendmsg(). This
>  -               * needs to be done before sctp_assoc_add_peer() as that will
>  -               * set up the initial packet that needs to account for any
>  -               * security ip options (CIPSO/CALIPSO) added to the packet.
>  -               */
>  -              af = sctp_get_af_specific(to.sa.sa_family);
>  -              if (!af) {
>  -                      err = -EINVAL;
>  -                      goto out_unlock;
>  +                      dlen = sizeof(struct in6_addr);
>  +                      daddr->v6.sin6_family = AF_INET6;
>  +                      daddr->v6.sin6_port = htons(asoc->peer.port);
>  +                      memcpy(&daddr->v6.sin6_addr, CMSG_DATA(cmsg), dlen);
>                 }
>  -              err = security_sctp_bind_connect(sk, SCTP_SENDMSG_CONNECT,
>  -                                               (struct sockaddr *)&to,
>  -                                               af->sockaddr_len);
>  -              if (err < 0)
>  -                      goto out_unlock;
>  +              err = sctp_verify_addr(sk, daddr, sizeof(*daddr));
>  +              if (err)
>  +                      goto free;
>
>  -              new_asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
>  -              if (!new_asoc) {
>  -                      err = -ENOMEM;
>  -                      goto out_unlock;
>  -              }
>  -              asoc = new_asoc;
>  -              err = sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL);
>  -              if (err < 0) {
>  -                      err = -ENOMEM;
>  -                      goto out_free;
>  +              old = sctp_endpoint_lookup_assoc(ep, daddr, &transport);
>  +              if (old && old != asoc) {
>  +                      if (old->state >= SCTP_STATE_ESTABLISHED)
>  +                              err = -EISCONN;
>  +                      else
>  +                              err = -EALREADY;
>  +                      goto free;
>                 }
>
>  -              /* If the SCTP_INIT ancillary data is specified, set all
>  -               * the association init values accordingly.
>  -               */
>  -              if (sinit) {
>  -                      if (sinit->sinit_num_ostreams) {
>  -                              __u16 outcnt = sinit->sinit_num_ostreams;
>  -
>  -                              asoc->c.sinit_num_ostreams = outcnt;
>  -                              /* outcnt has been changed, so re-init stream */
>  -                              err = sctp_stream_init(&asoc->stream, outcnt, 0,
>  -                                                     GFP_KERNEL);
>  -                              if (err)
>  -                                      goto out_free;
>  -                      }
>  -                      if (sinit->sinit_max_instreams) {
>  -                              asoc->c.sinit_max_instreams =
>  -                                      sinit->sinit_max_instreams;
>  -                      }
>  -                      if (sinit->sinit_max_attempts) {
>  -                              asoc->max_init_attempts
>  -                                      = sinit->sinit_max_attempts;
>  -                      }
>  -                      if (sinit->sinit_max_init_timeo) {
>  -                              asoc->max_init_timeo =
>  -                               msecs_to_jiffies(sinit->sinit_max_init_timeo);
>  -                      }
>  +              if (sctp_endpoint_is_peeled_off(ep, daddr)) {
>  +                      err = -EADDRNOTAVAIL;
>  +                      goto free;
>                 }
>
>  -              /* Prime the peer's transport structures.  */
>  -              transport = sctp_assoc_add_peer(asoc, &to, GFP_KERNEL, SCTP_UNKNOWN);
>  +              transport = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL,
>  +                                              SCTP_UNKNOWN);
>                 if (!transport) {
>                         err = -ENOMEM;
>  -                      goto out_free;
>  +                      goto free;
>                 }
>         }
>



-- 
paul moore
www.paul-moore.com