On Sat, Jan 21, 2017 at 07:31:20AM +0100, Claudio Jeker wrote:
> Next piece of the puzzle. Cleanup the error handling a bit.
> If the route message is not valid (syntactically or also because it
> references bad things) fail without broadcasting this message to all
> listeners. So make sure that until the info struct has been checked we only
> use goto fail instead of goto flush.
> While there remove some redundant checks which are not needed.
> 
> OK?

OK bluhm@

> -- 
> :wq Claudio
> 
> Index: net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.215
> diff -u -p -r1.215 rtsock.c
> --- net/rtsock.c      21 Jan 2017 03:44:46 -0000      1.215
> +++ net/rtsock.c      21 Jan 2017 06:23:02 -0000
> @@ -488,7 +488,6 @@ route_output(struct mbuf *m, ...)
>       so = va_arg(ap, struct socket *);
>       va_end(ap);
>  
> -     info.rti_info[RTAX_DST] = NULL; /* for error handling (goto flush) */
>       if (m == NULL || ((m->m_len < sizeof(int32_t)) &&
>           (m = m_pullup(m, sizeof(int32_t))) == 0))
>               return (ENOBUFS);
> @@ -555,10 +554,10 @@ route_output(struct mbuf *m, ...)
>       if (!rtable_exists(tableid)) {
>               if (rtm->rtm_type == RTM_ADD) {
>                       if ((error = rtable_add(tableid)) != 0)
> -                             goto flush;
> +                             goto fail;
>               } else {
>                       error = EINVAL;
> -                     goto flush;
> +                     goto fail;
>               }
>       }
>  
> @@ -598,7 +597,7 @@ route_output(struct mbuf *m, ...)
>           info.rti_info[RTAX_GATEWAY]->sa_family >= AF_MAX) ||
>           info.rti_info[RTAX_GENMASK] != NULL) {
>               error = EINVAL;
> -             goto flush;
> +             goto fail;
>       }
>  #ifdef MPLS
>       info.rti_mpls = rtm->rtm_mpls;
> @@ -610,6 +609,11 @@ route_output(struct mbuf *m, ...)
>               info.rti_flags |= RTF_LLINFO;
>       }
>  
> +     /*
> +      * Do not use goto flush before this point since the message itself
> +      * may be not consistent and could cause unexpected behaviour in other
> +      * userland clients. Use goto fail instead.
> +      */
>       switch (rtm->rtm_type) {
>       case RTM_ADD:
>               if (info.rti_info[RTAX_GATEWAY] == NULL) {
> @@ -647,11 +651,6 @@ route_output(struct mbuf *m, ...)
>               }
>               break;
>       case RTM_DELETE:
> -             if (!rtable_exists(tableid)) {
> -                     error = EAFNOSUPPORT;
> -                     goto flush;
> -             }
> -
>               rt = rtable_lookup(tableid, info.rti_info[RTAX_DST],
>                   info.rti_info[RTAX_NETMASK], info.rti_info[RTAX_GATEWAY],
>                   prio);
> @@ -690,10 +689,6 @@ route_output(struct mbuf *m, ...)
>                       goto report;
>               break;
>       case RTM_GET:
> -             if (!rtable_exists(tableid)) {
> -                     error = EAFNOSUPPORT;
> -                     goto flush;
> -             }
>               rt = rtable_lookup(tableid, info.rti_info[RTAX_DST],
>                   info.rti_info[RTAX_NETMASK], info.rti_info[RTAX_GATEWAY],
>                   prio);
> @@ -764,11 +759,6 @@ report:
>               break;
>       case RTM_CHANGE:
>       case RTM_LOCK:
> -             if (!rtable_exists(tableid)) {
> -                     error = EAFNOSUPPORT;
> -                     goto flush;
> -             }
> -
>               rt = rtable_lookup(tableid, info.rti_info[RTAX_DST],
>                   info.rti_info[RTAX_NETMASK], info.rti_info[RTAX_GATEWAY],
>                   prio);

Reply via email to