diff options
| -rw-r--r-- | zebra/ChangeLog | 9 | ||||
| -rw-r--r-- | zebra/kernel_socket.c | 127 |
2 files changed, 127 insertions, 9 deletions
diff --git a/zebra/ChangeLog b/zebra/ChangeLog index e4547f67..0bf262fb 100644 --- a/zebra/ChangeLog +++ b/zebra/ChangeLog @@ -1,3 +1,12 @@ +2004-01-05 Greg Troxel <gdt@ahi.ir.bbn.com> + + * kernel_socket.c (kernel_read): Look up interfaces by index + first, so that state changes which do not include a sockaddr_dl + now work. Add many sanity checks. In + particular, do not assume that a sockaddr_dl follows a message + without checking the ifm_addrs flags, and do not trust the length + in a sockaddr_dl. Add/clarify many comments. + 2003-12-03 Greg Troxel <gdt@poblano.ir.bbn.com> * rtadv.c: reorder includes to avoid compiler warning (define diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index 6950d680..97953ac1 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -211,7 +211,11 @@ ifan_read (struct if_announcemsghdr *ifan) } #endif /* RTM_IFANNOUNCE */ -/* Interface adding function called from interface_list. */ +/* + * Handle struct if_msghdr obtained from reading routing socket or + * sysctl (from interface_list). There may or may not be sockaddrs + * present after the header. + */ int ifm_read (struct if_msghdr *ifm) { @@ -219,6 +223,17 @@ ifm_read (struct if_msghdr *ifm) struct sockaddr_dl *sdl = NULL; char ifname[IFNAMSIZ]; + /* paranoia: sanity check structure */ + if (ifm->ifm_msglen < sizeof(struct if_msghdr)) + { + zlog_err ("ifm_read: ifm->ifm_msglen %d too short\n", + ifm->ifm_msglen); + return -1; + } + + /* + * Check for a sockaddr_dl following the message. + */ #ifdef SUNOS_5 int i; struct sockaddr *sa; @@ -246,33 +261,113 @@ ifm_read (struct if_msghdr *ifm) } } } + /* + * After here, If RTA_IFP was set in ifm_addrs, sdl should point to + * the sockaddr_dl. + */ #else - sdl = (struct sockaddr_dl *)(ifm + 1); + /* sockaddrs_present? */ + if (ifm->ifm_addrs) + { + if (ifm->ifm_addrs == RTA_IFP) + { + /* just the one we want */ + sdl = (struct sockaddr_dl *)(ifm + 1); + } + else + { + /* + * Not strictly an error, but more complicated parsing than + * is implemented is required to handle this case. + */ + zlog_err ("ifm_read: addrs %x != RTA_IFP (unhandled, ignoring)\n", + ifm->ifm_addrs); + return -1; + } + } + /* + * Past here, either ifm_addrs == 0 or ifm_addrs == RTA_IFP and sdl + * points to a RTA_IFP sockaddr. + */ #endif + /* Check that sdl, if present, is actually a sockaddr_dl before use. */ + if (sdl != NULL) + switch (sdl->sdl_family) + { + case AF_LINK: + /* Standard AF for link-layer address sockaddrs. */ + case AF_DLI: + /* + * XXX Comment in NetBSD net/if_dl.h says AF_DLI, but this + * seems wrong. Accept it for now. + */ + break; + + default: + zlog_err ("ifm_read: sockaddr_dl bad AF %d\n", + sdl->sdl_family); + return -1; + } + /* - * Check if ifp already exists. If the interface has already been specified - * in the conf file, but is just getting created, we would have an - * entry in the iflist with incomplete data (e.g., ifindex == -1), - * so we lookup on name. + * Look up on ifindex. This is useful if this is an up/down + * notification for an interface of which we are already aware. + * (This happens on NetBSD 1.6.2, for example.) */ - if (sdl != NULL) + if (ifp == NULL) + ifp = if_lookup_by_index (ifm->ifm_index); + + + /* + * If lookup by index was unsuccessful and we have a name, try + * looking up by name. Interfaces specified in the configuration + * file for which the ifindex has not been determined will have + * ifindex == -1, and such interfaces are found by this search, and + * then their ifindex values can be filled in. + */ + if (ifp != NULL && sdl != NULL) { + /* + * paranoia: sanity check name length. nlen does not include + * trailing zero, but IFNAMSIZ max length does. + */ + if (sdl->sdl_nlen >= IFNAMSIZ) + { + zlog_err ("ifm_read: illegal sdl_nlen %d\n", sdl->sdl_nlen); + return -1; + } + memcpy (ifname, sdl->sdl_data, sdl->sdl_nlen); ifname[sdl->sdl_nlen] = '\0'; ifp = if_lookup_by_name (ifname); } + /* + * If ifp does not exist or has an invalid index (-1), create or + * fill in an interface. + */ if ((ifp == NULL) || (ifp->ifindex == -1)) { /* Check interface's address.*/ if (! (ifm->ifm_addrs & RTA_IFP)) { - zlog_warn ("There must be RTA_IFP address for ifindex %d\n", + zlog_warn ("Interface index %d (new) missing RTA_IFP sockaddr\n", ifm->ifm_index); return -1; } + /* + * paranoia: sdl-finding code above guarantees that sdl is + * non-NULL and valid if RTA_IFP is set in ifm_addrs, so this + * check is in theory not necessary. + */ + if (sdl == NULL) + { + zlog_warn ("ifm_read: no sockaddr_dl present"); + return -1; + } + if (ifp == NULL) ifp = if_create (sdl->sdl_data, sdl->sdl_nlen); @@ -291,13 +386,20 @@ ifm_read (struct if_msghdr *ifm) zlog_warn ("sockaddr_dl->sdl_family is not AF_LINK"); return -1; } + /* XXX sockaddr_dl can be larger than base structure */ memcpy (&ifp->sdl, sdl, sizeof (struct sockaddr_dl)); if_add_update (ifp); } else + /* + * Interface structure exists. Adjust stored flags from + * notification. If interface has up->down or down->up + * transition, call state change routines (to adjust routes, + * notify routing daemons, etc.). (Other flag changes are stored + * but apparently do not trigger action.) + */ { - /* There is a case of promisc, allmulti flag modification. */ if (if_is_up (ifp)) { ifp->flags = ifm->ifm_flags; @@ -824,6 +926,13 @@ kernel_read (struct thread *thread) rtm = &buf.r.rtm; + if (rtm->rtm_msglen != nbytes) + { + zlog_warn ("kernel_read: rtm->rtm_msglen %d, nbytes %d, type %d\n", + rtm->rtm_msglen, nbytes, rtm->rtm_type); + return -1; + } + switch (rtm->rtm_type) { case RTM_ADD: |