On 09/30/2016 04:54 PM, David R. Hedges wrote: > The interface name listed by /proc/net/ipv6_route is a char > buf[IFNAMSIZ], which can be longer than 10.
Specifically, they can be 15 bytes long (16 plus a null terminator): /usr/include/linux/if.h:#define IFNAMSIZ 16 Which matches the FSF's ludicrous: /usr/include/net/if.h:#define IF_NAMESIZE 16 /usr/include/net/if.h:# define IFNAMSIZ IF_NAMESIZE > Change our iface buffer to that same length, and fscanf a maximum string > of that many characters (including \0). There's a reason route's still in pending, I haven't done a proper cleanup pass on it yet. (Largely because I need to set up network tests, which requires a known vm test environment, and aboriginal linux has been in pieces for about a year now due to toolchain issues. It's on the todo list...) > Also, try to more gracefully handle unexpectedly long lines from > /proc/net/ipv6_route in fscanf by matching (but not assigning) any > trailing characters before the newline, otherwise they're the starting > point for the next iteration. Ditto. (Isn't fscanf supposed to match arbitrary runs of whitespace any time you feed it whitespace automatically? Although I admit trusting /proc output to be sane because it requires root access to screw this input up.) > Here's an ipv6_route test case that demonstrated the problem: > > 260777000000001f000000013f8384c0 80 00000000000000000000000000000000 00 > fe800000000000006512d4c76293bf99 00000000 00000001 00000005 01050003 > rmnet_data0 > 260777000000001f00000001d1be6a0a 80 00000000000000000000000000000000 00 > fe800000000000006512d4c76293bf99 00000000 00000002 0000018d 01050003 > rmnet_data0 > 260777000000001f00000001d83ac0c2 80 00000000000000000000000000000000 00 > fe800000000000006512d4c76293bf99 00000000 00000001 00000002 01050003 > rmnet_data0 This would come from /proc/net/ipv6_route? (I wonder why there are three ip6_* files in that directory but that one's (the only) ipv6_*?) That's a length 11 name, still 5 bytes left... > toys/pending/route.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) Let's see... > diff --git a/toys/pending/route.c b/toys/pending/route.c > index 0126dc9..f0a1d54 100644 > --- a/toys/pending/route.c > +++ b/toys/pending/route.c > @@ -24,6 +24,8 @@ config ROUTE > #define FOR_route > #include "toys.h" > #include <net/route.h> > +#include <linux/if.h> //for IFNAMSIZ Already defined by net/if.h which is #included by toys.h, and posix says that's the right header for it: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/net_if.h.html > +#include <sys/cdefs.h> //for __STRING(x) A) Really shouldn't be needed here. B) This header is aggressively nonstandard and the fact musl intentionally doesn't ship it is a FAQ on their website: http://wiki.musl-libc.org/wiki/FAQ#Q:_I.27m_trying_to_compile_something_against_musl_and_I_get_error_messages_about_sys.2Fcdefs.h > GLOBALS( > char *family; > @@ -32,6 +34,8 @@ GLOBALS( > #define DEFAULT_PREFIXLEN 128 > #define INVALID_ADDR 0xffffffffUL > #define IPV6_ADDR_LEN 40 //32 + 7 (':') + 1 ('\0') > +#define STRINGIFY(x) __STRING(x) > +#define _S_IFNAMSIZ STRINGIFY( IFNAMSIZ) If you're going to do that why not just define STRINGIFY(x) #x (which is the c99 preprocessor feature that does this). And we still shouldn't need it. > struct _arglist { > char *arg; > @@ -393,7 +397,7 @@ static void ipv6_addr_formating(char *ptr, char *addr) > > static void display_routes6(void) > { > - char iface[16] = {0,}, ipv6_dest_addr[41]; > + char iface[IFNAMSIZ] = {0,}, ipv6_dest_addr[41]; It's... the same? It's literally the same value? It's been hardwired to 16 in the kernel since June 17, 1993: Linux-0.99.10 (June 7, 1993) People finally gave up on net-1, Ross Biro grew tired of the flames, and net-2 appears with Fred van Kempen as maintainer. This is the big switch-over version. Did I mention I have a kernel git repo that goes back to 0.0.1? Handy thing to have, you can: wget http://landley.net/kdocs/local/linux-fullhist.tar.bz2 tar xvjf linux-fullhist.tar.bz2 cd linux-fullhist git checkout -f git pull And it should be up to date and ready to use, and you can "git annotate" all the way back. (In this tree, linux-0.99.10 is commit 54c93c05f69e.) Anyway, I agree using the symbol is cleaner, but it shouldn't change the behavior? > char ipv6_src_addr[41], flag_val[10], buf2[INET6_ADDRSTRLEN]; > int prefixlen, metric, use, refcount, flag, items = 0; > unsigned char buf[sizeof(struct in6_addr)]; > @@ -403,8 +407,8 @@ static void display_routes6(void) > xprintf("Kernel IPv6 routing table\n" > "%-43s%-40s Flags Metric Ref Use Iface\n", "Destination", "Next > Hop"); > > - while ((items = fscanf(fp, "%32s%x%*s%*x%32s%x%x%x%x%10s\n", > ipv6_dest_addr+8, > - &prefixlen, ipv6_src_addr+8, &metric, &use, &refcount, &flag, There's your bug, that %10s should be %16s. > + while ((items = fscanf(fp, "%32s%x%*s%*x%32s%x%x%x%x%" _S_IFNAMSIZ > "s%*[^\n]\n", ipv6_dest_addr+8, > + &prefixlen, ipv6_src_addr+8, &metric, &use, &refcount, &flag, Except it needs to be 15 because it doesn't include the length of the null terminator. Personally I'd just hardware 15 in here (and use 15 in the variable definition). I really doubt that if we built on freebsd or macos (do solaris and aix still exist in a meaningful sense?) without a Linux emulation layer (which everything but macos has anyway), we'd get the right /proc file with the same data format anyway, so the name size constant changing (which would break the ABI, something Linux is _proud_ it hasn't done since 0.0.1, modulo Greg KH and Kay Sievers being stupid with /proc and /sys and Kay at least got chewed out about it by Linus) would be the least of our problems. > iface)) == 8) > { > if (!(flag & RTF_UP)) continue; //skip down interfaces. > -- > 2.7.4 > Thanks for the bug report. For now I'm just changing the value in the fscanf to 15. My larger todo item here is figuring out what to do about ipv6. (Having to say -A ipv6 to enable ipv6 mode is kinda silly, in theory it should autodetect and produce both kinds of output? In practice that would probably break existing users and the field headers are different. Sigh...) Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
