On Thu, Feb 05, 2015 at 07:28:45PM +0100, Ben Noordhuis wrote: > A small test program follows: > > #include <sys/socket.h> > #include <netinet/in.h> > #include <arpa/inet.h> > #include <stddef.h> > > int main(void) { > int fd; > struct ip_mreq m4; > struct ipv6_mreq m6; > fd = socket(AF_INET6, SOCK_DGRAM, 0); > inet_aton("224.0.0.2", &m4.imr_multiaddr); > inet_aton("0.0.0.0", &m4.imr_interface); > inet_pton(AF_INET6, "ff01::c", &m6.ipv6mr_multiaddr); > m6.ipv6mr_interface = 1; > setsockopt(fd, SOL_IP, IP_ADD_MEMBERSHIP, &m4, 1); > setsockopt(fd, SOL_IP, IP_DROP_MEMBERSHIP, &m4, 1); > setsockopt(fd, SOL_IP, IP_ADD_MEMBERSHIP, &m4, sizeof(m4)); > setsockopt(fd, SOL_IP, IP_DROP_MEMBERSHIP, &m4, sizeof(m4)); > setsockopt(fd, SOL_IPV6, IPV6_ADD_MEMBERSHIP, &m6, 1); > setsockopt(fd, SOL_IPV6, IPV6_DROP_MEMBERSHIP, &m6, 1); > setsockopt(fd, SOL_IPV6, IPV6_ADD_MEMBERSHIP, &m6, sizeof(m6)); > setsockopt(fd, SOL_IPV6, IPV6_DROP_MEMBERSHIP, &m6, sizeof(m6)); > m6.ipv6mr_interface = 42; > setsockopt(fd, SOL_IPV6, IPV6_ADD_MEMBERSHIP, &m6, sizeof(m6)); > setsockopt(fd, SOL_IPV6, IPV6_DROP_MEMBERSHIP, &m6, sizeof(m6)); > return 0; > } > > And prints the following output when traced: > > setsockopt(3, SOL_IP, IP_ADD_MEMBERSHIP, "\340", 1) > = -1 EINVAL (Invalid argument) > setsockopt(3, SOL_IP, IP_DROP_MEMBERSHIP, "\340", 1) > = -1 EINVAL (Invalid argument) > setsockopt(3, SOL_IP, IP_ADD_MEMBERSHIP, > {imr_multiaddr=inet_addr("224.0.0.2"), > imr_interface=inet_addr("0.0.0.0")}, 8) = 0 > setsockopt(3, SOL_IP, IP_DROP_MEMBERSHIP, > {imr_multiaddr=inet_addr("224.0.0.2"), > imr_interface=inet_addr("0.0.0.0")}, 8) = 0 > setsockopt(3, SOL_IPV6, IPV6_ADD_MEMBERSHIP, "\377", 1) > = -1 EINVAL (Invalid argument) > setsockopt(3, SOL_IPV6, IPV6_DROP_MEMBERSHIP, "\377", 1) > = -1 EINVAL (Invalid argument) > setsockopt(3, SOL_IPV6, IPV6_ADD_MEMBERSHIP, > {ipv6mr_multiaddr=inet_pton("ff01::c"), > ipv6mr_interface=if_nametoindex("lo")}, 20) = 0 > setsockopt(3, SOL_IPV6, IPV6_DROP_MEMBERSHIP, > {ipv6mr_multiaddr=inet_pton("ff01::c"), > ipv6mr_interface=if_nametoindex("lo")}, 20) = 0 > setsockopt(3, SOL_IPV6, IPV6_ADD_MEMBERSHIP, > {ipv6mr_multiaddr=inet_pton("ff01::c"), > ipv6mr_interface=42}, 20) = -1 ENODEV (No such device) > setsockopt(3, SOL_IPV6, IPV6_DROP_MEMBERSHIP, > {ipv6mr_multiaddr=inet_pton("ff01::c"), > ipv6mr_interface=42}, 20) = -1 EADDRNOTAVAIL > (Cannot assign requested address)
This is a good candidate for a new test in tests/ directory, isn't it? > * net.c (sys_setsockopt): decode IP_ADD_MEMBERSHIP, IP_DROP_MEMBERSHIP, > IPV6_ADD_MEMBERSHIP and IPV6_DROP_MEMBERSHIP arguments. > > Signed-off-by: Ben Noordhuis <i...@bnoordhuis.nl> > --- > net.c | 85 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 85 insertions(+) > > diff --git a/net.c b/net.c > index 49185dc..f55c6af 100644 > --- a/net.c > +++ b/net.c > @@ -1325,6 +1325,73 @@ sys_getsockopt(struct tcb *tcp) > return 0; > } > > +static void > +print_interface(unsigned int index) > +{ > +#ifdef HAVE_IF_INDEXTONAME > + char buf[IFNAMSIZ + 1]; > + if (if_indextoname(index, buf) == buf) { if_indextoname() returns NULL on error, so the check could be made simpler: if (if_indextoname(index, buf)) { > + tprints("if_nametoindex("); > + print_quoted_string(buf, sizeof(buf), QUOTE_0_TERMINATED); > + tprints(")"); > + return; > + } > +#endif > + tprintf("%u", index); > +} I'd put this new function before printsock(), as the latter is going to call it after your second patch. > + > +#ifdef IP_ADD_MEMBERSHIP > +static void > +print_mreq(struct tcb *tcp, long addr, int len) > +{ > + struct ip_mreq mreq; > + if (len == sizeof(mreq) && umove(tcp, addr, &mreq) == 0) { > + tprints("{imr_multiaddr=inet_addr("); > + print_quoted_string(inet_ntoa(mreq.imr_multiaddr), > + 16, QUOTE_0_TERMINATED); > + tprints("), imr_interface=inet_addr("); > + print_quoted_string(inet_ntoa(mreq.imr_interface), > + 16, QUOTE_0_TERMINATED); > + tprints(")}"); > + } > + else { > + printstr(tcp, addr, len); > + } > +} Is there any use to print the address with printstr if length is not sizeof(ip_mreq) or umove has failed? Other sockopt parsers in such situations just print the address in hex. > +#endif /* IP_ADD_MEMBERSHIP */ > + > +#ifdef IPV6_ADD_MEMBERSHIP > +static void > +print_mreq6(struct tcb *tcp, long addr, int len) > +{ > +#ifdef HAVE_INET_NTOP > + struct ipv6_mreq mreq; > + const struct in6_addr *in6; > + char address[INET6_ADDRSTRLEN]; > + > + if (len != sizeof(mreq)) > + goto fail; > + > + if (umove(tcp, addr, &mreq) < 0) > + goto fail; Not sure about falling back to printstr here as well. > + in6 = &mreq.ipv6mr_multiaddr; > + if (inet_ntop(AF_INET6, in6, address, sizeof(address)) != address) > + goto fail; A chance that inet_ntop would fail in this case is very unlikely indeed. Anyway, the string is already fetched, we could use something like print_quoted_string(&mreq, sizeof(mreq), 0); [...] > @@ -1445,6 +1518,18 @@ print_setsockopt(struct tcb *tcp, int level, int name, > long addr, int len) > goto done; > #endif /* MCAST_JOIN_GROUP */ > } > + break; Yes, thanks for fixing my bugs. -- ldv
pgpkBhPTqxgl2.pgp
Description: PGP signature
------------------------------------------------------------------------------ Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel