With prodding from Todd, I tested the diff on my macppc and my alpha.
Before the diff, running `ppp default` as root would hang before
presenting me with the interactive prompt. With the diff, it
immediately goes into interactive mode and presents me with the prompt.
Unfortunately, I don't have another way to test.
-ME
On Wed, Jul 01, 2009 at 11:23:20AM -0500, Todd T. Fries wrote:
> Guys,
>
> I tested this and it seems ppp in the tree is busted to the point of not
> working without this diff.
>
> If you use ppp please test current snaps to confirm it is busted then
> apply claudio's diff below and test again.
>
> If you use ppp and do not test, do not be surprised if it does not work in
> the next release.
>
> Thanks,
>
> Penned by Claudio Jeker on 20090630 17:05.28, we have:
> | So ppp(8) did insane routing message handling in its sysctl handlers. The
> | worst thing about them are that their actually not needed and better
> | replaced with libc functions (getifaddrs and if_nametoindex).
> |
> | This diff is not haevily tested (my last ppp usage is years ago) so I'm
> | hopeing people with ppp(8) issues could give this a whirl and see if it
> | fixes the problems.
> | --
> | :wq Claudio
> |
> | Index: ppp/arp.c
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/arp.c,v
> | retrieving revision 1.15
> | diff -u -p -r1.15 arp.c
> | --- ppp/arp.c 6 May 2008 06:34:10 -0000 1.15
> | +++ ppp/arp.c 30 Jun 2009 14:52:53 -0000
> | @@ -38,6 +38,7 @@
> | #include <sys/un.h>
> |
> | #include <errno.h>
> | +#include <ifaddrs.h>
> | #include <stdio.h>
> | #include <stdlib.h>
> | #include <string.h>
> | @@ -229,93 +230,58 @@ int
> | arp_EtherAddr(int s, struct in_addr ipaddr, struct sockaddr_dl *hwaddr,
> | int verbose)
> | {
> | - int mib[6], skip;
> | - size_t needed;
> | - char *buf, *ptr, *end;
> | - struct if_msghdr *ifm;
> | - struct ifa_msghdr *ifam;
> | - struct sockaddr_dl *dl;
> | - struct sockaddr *sa[RTAX_MAX];
> | -
> | - mib[0] = CTL_NET;
> | - mib[1] = PF_ROUTE;
> | - mib[2] = 0;
> | - mib[3] = 0;
> | - mib[4] = NET_RT_IFLIST;
> | - mib[5] = 0;
> | -
> | - if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) {
> | - log_Printf(LogERROR, "arp_EtherAddr: sysctl: estimate: %s\n",
> | - strerror(errno));
> | - return 0;
> | - }
> | -
> | - if ((buf = malloc(needed)) == NULL)
> | - return 0;
> | + struct sockaddr_dl *dl = NULL;
> | + struct ifaddrs *ifa, *ifap;
> | + int skip = 1;
> |
> | - if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) {
> | - free(buf);
> | + if (getifaddrs(&ifap) != 0) {
> | + log_Printf(LogERROR, "arp_EtherAddr: getifaddrs: %s\n",
> strerror(errno));
> | return 0;
> | }
> | - end = buf + needed;
> |
> | - ptr = buf;
> | - while (ptr < end) {
> | - ifm = (struct if_msghdr *)ptr; /* On if_msghdr */
> | - if (ifm->ifm_type != RTM_IFINFO)
> | - break;
> | - ptr += ifm->ifm_msglen;
> | - if (ifm->ifm_version != RTM_VERSION)
> | - continue;
> | - dl = (struct sockaddr_dl *)(ifm + 1); /* Single _dl at end */
> | - skip = (ifm->ifm_flags & (IFF_UP | IFF_BROADCAST | IFF_POINTOPOINT |
> | + for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> | + if (ifa->ifa_addr->sa_family == AF_LINK) {
> | + dl = (struct sockaddr_dl *)ifa->ifa_addr;
> | + skip = (ifa->ifa_flags & (IFF_UP | IFF_BROADCAST | IFF_POINTOPOINT |
> | IFF_NOARP | IFF_LOOPBACK)) != (IFF_UP | IFF_BROADCAST);
> | - while (ptr < end) {
> | - ifam = (struct ifa_msghdr *)ptr; /* Next ifa_msghdr (alias) */
> | - if (ifam->ifam_type != RTM_NEWADDR) /* finished ? */
> | - break;
> | - ptr += ifam->ifam_msglen;
> | - if (ifam->ifam_version != RTM_VERSION)
> | - continue;
> | - if (skip || (ifam->ifam_addrs & (RTA_NETMASK|RTA_IFA)) !=
> | - (RTA_NETMASK|RTA_IFA))
> | - continue;
> | - /* Found a candidate. Do the addresses match ? */
> | - if (log_IsKept(LogDEBUG) &&
> | - ptr == (char *)ifm + ifm->ifm_msglen + ifam->ifam_msglen)
> | - log_Printf(LogDEBUG, "%.*s interface is a candidate for proxy\n",
> | - dl->sdl_nlen, dl->sdl_data);
> | -
> | - iface_ParseHdr(ifam, sa);
> | -
> | - if (sa[RTAX_IFA]->sa_family == AF_INET) {
> | - struct sockaddr_in *ifa, *netmask;
> | -
> | - ifa = (struct sockaddr_in *)sa[RTAX_IFA];
> | - netmask = (struct sockaddr_in *)sa[RTAX_NETMASK];
> | -
> | - if (log_IsKept(LogDEBUG)) {
> | - char a[16];
> | -
> | - strncpy(a, inet_ntoa(netmask->sin_addr), sizeof a - 1);
> | - a[sizeof a - 1] = '\0';
> | - log_Printf(LogDEBUG, "Check addr %s, mask %s\n",
> | - inet_ntoa(ifa->sin_addr), a);
> | - }
> | -
> | - if ((ifa->sin_addr.s_addr & netmask->sin_addr.s_addr) ==
> | - (ipaddr.s_addr & netmask->sin_addr.s_addr)) {
> | - log_Printf(verbose ? LogPHASE : LogDEBUG,
> | - "Found interface %.*s for %s\n", dl->sdl_nlen,
> | - dl->sdl_data, inet_ntoa(ipaddr));
> | - memcpy(hwaddr, dl, dl->sdl_len);
> | - free(buf);
> | - return 1;
> | - }
> | + continue;
> | + }
> | + if (skip)
> | + /* Skip unusable interface */
> | + continue;
> | +
> | + /* Found a candidate. Do the addresses match ? */
> | + if (log_IsKept(LogDEBUG))
> | + log_Printf(LogDEBUG, "%.*s interface is a candidate for proxy\n",
> | + dl->sdl_nlen, dl->sdl_data);
> | +
> | + if (ifa->ifa_addr->sa_family == AF_INET) {
> | + struct sockaddr_in *addr, *netmask;
> | +
> | + addr = (struct sockaddr_in *)ifa->ifa_addr;
> | + netmask = (struct sockaddr_in *)ifa->ifa_netmask;
> | +
> | + if (log_IsKept(LogDEBUG)) {
> | + char a[16];
> | +
> | + strncpy(a, inet_ntoa(netmask->sin_addr), sizeof a - 1);
> | + a[sizeof a - 1] = '\0';
> | + log_Printf(LogDEBUG, "Check addr %s, mask %s\n",
> | + inet_ntoa(addr->sin_addr), a);
> | + }
> | +
> | + if ((addr->sin_addr.s_addr & netmask->sin_addr.s_addr) ==
> | + (ipaddr.s_addr & netmask->sin_addr.s_addr)) {
> | + log_Printf(verbose ? LogPHASE : LogDEBUG,
> | + "Found interface %.*s for %s\n", dl->sdl_nlen,
> | + dl->sdl_data, inet_ntoa(ipaddr));
> | + memcpy(hwaddr, dl, dl->sdl_len);
> | + freeifaddrs(ifap);
> | + return 1;
> | }
> | }
> | }
> | - free(buf);
> | + freeifaddrs(ifap);
> |
> | return 0;
> | }
> | Index: ppp/iface.c
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/iface.c,v
> | retrieving revision 1.28
> | diff -u -p -r1.28 iface.c
> | --- ppp/iface.c 25 Jun 2009 15:59:28 -0000 1.28
> | +++ ppp/iface.c 30 Jun 2009 14:54:08 -0000
> | @@ -44,6 +44,7 @@
> | #include <sys/un.h>
> |
> | #include <errno.h>
> | +#include <ifaddrs.h>
> | #include <string.h>
> | #include <stdarg.h>
> | #include <stdio.h>
> | @@ -90,114 +91,64 @@ static const struct in6_addr in6mask128
> | struct iface *
> | iface_Create(const char *name)
> | {
> | - int mib[6], maxtries, err;
> | - size_t needed, namelen;
> | - char *buf, *ptr, *end;
> | - struct if_msghdr *ifm;
> | - struct ifa_msghdr *ifam;
> | + size_t namelen;
> | struct sockaddr_dl *dl;
> | - struct sockaddr *sa[RTAX_MAX];
> | + struct ifaddrs *ifap, *ifa;
> | struct iface *iface;
> | struct iface_addr *addr;
> |
> | - mib[0] = CTL_NET;
> | - mib[1] = PF_ROUTE;
> | - mib[2] = 0;
> | - mib[3] = 0;
> | - mib[4] = NET_RT_IFLIST;
> | - mib[5] = 0;
> | -
> | - maxtries = 20;
> | - err = 0;
> | - do {
> | - if (maxtries-- == 0 || (err && err != ENOMEM)) {
> | - fprintf(stderr, "iface_Create: sysctl: %s\n", strerror(err));
> | - return NULL;
> | - }
> | -
> | - if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) {
> | - fprintf(stderr, "iface_Create: sysctl: estimate: %s\n",
> | - strerror(errno));
> | - return NULL;
> | - }
> | -
> | - if ((buf = (char *)malloc(needed)) == NULL) {
> | - fprintf(stderr, "iface_Create: malloc failed: %s\n",
> strerror(errno));
> | - return NULL;
> | - }
> | -
> | - if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) {
> | - err = errno;
> | - free(buf);
> | - buf = NULL;
> | - }
> | - } while (buf == NULL);
> | + if (getifaddrs(&ifap) != 0) {
> | + fprintf(stderr, "iface_Create: getifaddrs: %s\n", strerror(errno));
> | + return NULL;
> | + }
> |
> | - ptr = buf;
> | - end = buf + needed;
> | iface = NULL;
> | namelen = strlen(name);
> |
> | - while (ptr < end && iface == NULL) {
> | - ifm = (struct if_msghdr *)ptr; /* On if_msghdr */
> | - if (ifm->ifm_version != RTM_VERSION)
> | + for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> | + if (strcmp(name, ifa->ifa_name))
> | continue;
> | - if (ifm->ifm_type != RTM_IFINFO)
> | - break;
> | - dl = (struct sockaddr_dl *)(ifm + 1); /* Single _dl at end */
> | - if (dl->sdl_nlen == namelen && !strncmp(name, dl->sdl_data, namelen)) {
> | + if (ifa->ifa_addr->sa_family == AF_LINK) {
> | + dl = (struct sockaddr_dl *)ifa->ifa_addr;
> | iface = (struct iface *)malloc(sizeof *iface);
> | if (iface == NULL) {
> | fprintf(stderr, "iface_Create: malloc: %s\n", strerror(errno));
> | + freeifaddrs(ifap);
> | return NULL;
> | }
> | iface->name = strdup(name);
> | - iface->index = ifm->ifm_index;
> | - iface->flags = ifm->ifm_flags;
> | + iface->index = if_nametoindex(name);
> | + iface->flags = ifa->ifa_flags;
> | iface->mtu = 0;
> | iface->addrs = 0;
> | iface->addr = NULL;
> | }
> | - ptr += ifm->ifm_msglen; /* First
> ifa_msghdr */
> | - for (; ptr < end; ptr += ifam->ifam_msglen) {
> | - ifam = (struct ifa_msghdr *)ptr; /* Next if
> address */
> | -
> | - if (ifam->ifam_type != RTM_NEWADDR) /* finished this if */
> | - break;
> | - if (ifm->ifm_version != RTM_VERSION)
> | - continue;
> | -
> | - if (iface != NULL && ifam->ifam_addrs & RTA_IFA) {
> | - /* Found a configured interface ! */
> | - iface_ParseHdr(ifam, sa);
> |
> | - if (sa[RTAX_IFA] && (sa[RTAX_IFA]->sa_family == AF_INET
> | + if (ifa->ifa_addr->sa_family == AF_INET
> | #ifndef NOINET6
> | - || sa[RTAX_IFA]->sa_family == AF_INET6
> | + || ifa->ifa_addr->sa_family == AF_INET6
> | #endif
> | - )) {
> | - /* Record the address */
> | + ) {
> | + /* Record the address */
> |
> | - addr = (struct iface_addr *)
> | - realloc(iface->addr, (iface->addrs + 1) * sizeof
> iface->addr[0]);
> | - if (addr == NULL)
> | - break;
> | - iface->addr = addr;
> | -
> | - addr += iface->addrs;
> | - iface->addrs++;
> | -
> | - ncprange_setsa(&addr->ifa, sa[RTAX_IFA], sa[RTAX_NETMASK]);
> | - if (sa[RTAX_BRD])
> | - ncpaddr_setsa(&addr->peer, sa[RTAX_BRD]);
> | - else
> | - ncpaddr_init(&addr->peer);
> | - }
> | - }
> | + addr = (struct iface_addr *)
> | + realloc(iface->addr, (iface->addrs + 1) * sizeof iface->addr[0]);
> | + if (addr == NULL)
> | + break;
> | + iface->addr = addr;
> | +
> | + addr += iface->addrs;
> | + iface->addrs++;
> | +
> | + ncprange_setsa(&addr->ifa, ifa->ifa_addr, ifa->ifa_netmask);
> | + if (ifa->ifa_broadaddr)
> | + ncpaddr_setsa(&addr->peer, ifa->ifa_broadaddr);
> | + else
> | + ncpaddr_init(&addr->peer);
> | }
> | }
> |
> | - free(buf);
> | + freeifaddrs(ifap);
> |
> | return iface;
> | }
> | @@ -700,20 +651,4 @@ iface_Show(struct cmdargs const *arg)
> | }
> |
> | return 0;
> | -}
> | -
> | -void
> | -iface_ParseHdr(struct ifa_msghdr *ifam, struct sockaddr *sa[RTAX_MAX])
> | -{
> | - char *wp;
> | - int rtax;
> | -
> | - wp = (char *)(ifam + 1);
> | -
> | - for (rtax = 0; rtax < RTAX_MAX; rtax++)
> | - if (ifam->ifam_addrs & (1 << rtax)) {
> | - sa[rtax] = (struct sockaddr *)wp;
> | - wp += ROUNDUP(sa[rtax]->sa_len);
> | - } else
> | - sa[rtax] = NULL;
> | }
> | Index: ppp/iface.h
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/iface.h,v
> | retrieving revision 1.8
> | diff -u -p -r1.8 iface.h
> | --- ppp/iface.h 19 Aug 2001 23:22:17 -0000 1.8
> | +++ ppp/iface.h 30 Jun 2009 14:51:03 -0000
> | @@ -62,4 +62,3 @@ extern int iface_Show(struct cmdargs con
> | extern int iface_SetFlags(const char *, int);
> | extern int iface_ClearFlags(const char *, int);
> | extern void iface_Destroy(struct iface *);
> | -extern void iface_ParseHdr(struct ifa_msghdr *, struct sockaddr
> *[RTAX_MAX]);
> | Index: ppp/route.c
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/route.c,v
> | retrieving revision 1.37
> | diff -u -p -r1.37 route.c
> | --- ppp/route.c 25 Jun 2009 15:59:28 -0000 1.37
> | +++ ppp/route.c 30 Jun 2009 14:03:36 -0000
> | @@ -204,113 +204,15 @@ static int route_nifs = -1;
> | const char *
> | Index2Nam(int idx)
> | {
> | - /*
> | - * XXX: Maybe we should select() on the routing socket so that we can
> | - * notice interfaces that come & go (PCCARD support).
> | - * Or we could even support a signal that resets these so that
> | - * the PCCARD insert/remove events can signal ppp.
> | - */
> | - static char **ifs; /* Figure these out once */
> | - static int debug_done; /* Debug once */
> | + static char ifname[IF_NAMESIZE];
> | + char *ifn;
> |
> | - if (idx > route_nifs || (idx > 0 && ifs[idx-1] == NULL)) {
> | - int mib[6], have, had;
> | - size_t needed;
> | - char *buf, *ptr, *end;
> | - struct sockaddr_dl *dl;
> | - struct if_msghdr *ifm;
> | + ifn = if_indextoname(idx, ifname);
> |
> | - if (ifs) {
> | - free(ifs);
> | - ifs = NULL;
> | - route_nifs = 0;
> | - }
> | - debug_done = 0;
> | -
> | - mib[0] = CTL_NET;
> | - mib[1] = PF_ROUTE;
> | - mib[2] = 0;
> | - mib[3] = 0;
> | - mib[4] = NET_RT_IFLIST;
> | - mib[5] = 0;
> | -
> | - if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) {
> | - log_Printf(LogERROR, "Index2Nam: sysctl: estimate: %s\n",
> | - strerror(errno));
> | - return NumStr(idx, NULL, 0);
> | - }
> | - if ((buf = malloc(needed)) == NULL)
> | - return NumStr(idx, NULL, 0);
> | - if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) {
> | - free(buf);
> | - return NumStr(idx, NULL, 0);
> | - }
> | - end = buf + needed;
> | -
> | - have = 0;
> | - for (ptr = buf; ptr < end; ptr += ifm->ifm_msglen) {
> | - ifm = (struct if_msghdr *)ptr;
> | - if (ifm->ifm_version != RTM_VERSION)
> | - continue;
> | - if (ifm->ifm_type != RTM_IFINFO)
> | - continue;
> | - dl = (struct sockaddr_dl *)(ifm + 1);
> | - if (ifm->ifm_index > 0) {
> | - if (ifm->ifm_index > have) {
> | - char **newifs;
> | -
> | - had = have;
> | - have = ifm->ifm_index + 5;
> | - if (had)
> | - newifs = (char **)realloc(ifs, sizeof(char *) * have);
> | - else
> | - newifs = (char **)calloc(sizeof(char *), have);
> | - if (!newifs) {
> | - log_Printf(LogDEBUG, "Index2Nam: %s\n", strerror(errno));
> | - route_nifs = 0;
> | - if (ifs) {
> | - free(ifs);
> | - ifs = NULL;
> | - }
> | - free(buf);
> | - return NumStr(idx, NULL, 0);
> | - }
> | - ifs = newifs;
> | - memset(ifs + had, '\0', sizeof(char *) * (have - had));
> | - }
> | - if (ifs[ifm->ifm_index-1] == NULL) {
> | - ifs[ifm->ifm_index-1] = (char *)malloc(dl->sdl_nlen+1);
> | - if (ifs[ifm->ifm_index-1] == NULL)
> | - log_Printf(LogDEBUG, "Skipping interface %d: Out of memory\n",
> | - ifm->ifm_index);
> | - else {
> | - memcpy(ifs[ifm->ifm_index-1], dl->sdl_data, dl->sdl_nlen);
> | - ifs[ifm->ifm_index-1][dl->sdl_nlen] = '\0';
> | - if (route_nifs < ifm->ifm_index)
> | - route_nifs = ifm->ifm_index;
> | - }
> | - }
> | - } else if (log_IsKept(LogDEBUG))
> | - log_Printf(LogDEBUG, "Skipping out-of-range interface %d!\n",
> | - ifm->ifm_index);
> | - }
> | - free(buf);
> | - }
> | -
> | - if (log_IsKept(LogDEBUG) && !debug_done) {
> | - int f;
> | -
> | - log_Printf(LogDEBUG, "Found the following interfaces:\n");
> | - for (f = 0; f < route_nifs; f++)
> | - if (ifs[f] != NULL)
> | - log_Printf(LogDEBUG, " Index %d, name \"%s\"\n", f+1, ifs[f]);
> | - debug_done = 1;
> | - }
> | -
> | - if (idx < 1 || idx > route_nifs || ifs[idx-1] == NULL)
> | + if (idx < 1 || ifn == NULL)
> | return NumStr(idx, NULL, 0);
> |
> | - return ifs[idx-1];
> | + return ifn;
> | }
> |
> | void
>
> --
> Todd Fries .. [email protected]
>
> _____________________________________________
> | \ 1.636.410.0632 (voice)
> | Free Daemon Consulting, LLC \ 1.405.227.9094 (voice)
> | http://FreeDaemonConsulting.com \ 1.866.792.3418 (FAX)
> | "..in support of free software solutions." \ 250797 (FWD)
> | \
> \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
>
> 37E7 D3EB 74D0 8D66 A68D B866 0326 204E 3F42 004A
> http://todd.fries.net/pgp.txt
