On Sat, Jan 21, 2017 at 01:28:02AM +0100, Claudio Jeker wrote:
> On Fri, Jan 20, 2017 at 02:51:52AM +0100, Claudio Jeker wrote:
> > I sent this diff out some time ago and would really like to get this in.
> > This is one step on makeing rtsock.c less of a hornets nest.
> > This reduces the side effects in route_output and simplifies some other
> > bits as well. For example route_input is less variadic and simpler.
> >
>
> Here is just the route_input change. Which should be easier to OK.
> Changed a few things based on input from bluhm@
>
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?
--
: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);
