From: Ben N. <in...@bn...> - 2015-02-05 18:54:30
|
The first patch makes the setsockopt() option parser aware of IP_ADD_MEMBERSHIP and IP_DROP_MEMBERSHIP and its IPv6 variants. It also fixes a minor fall-through bug in the SOL_IP switch case. The second patch reduces the line count in net.c a little by reusing a function that is introduced in the first patch. net.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 96 insertions(+), 23 deletions(-) |
From: Ben N. <in...@bn...> - 2014-03-17 22:20:23
|
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 = 42; setsockopt(fd, SOL_IP, IP_ADD_MEMBERSHIP, &m4, 4); setsockopt(fd, SOL_IP, IP_DROP_MEMBERSHIP, &m4, 4); setsockopt(fd, SOL_IP, IP_ADD_MEMBERSHIP, &m4, sizeof(m4)); setsockopt(fd, SOL_IP, IP_DROP_MEMBERSHIP, &m4, sizeof(m4)); setsockopt(fd, SOL_IP, IPV6_ADD_MEMBERSHIP, &m6, sizeof(m6)); setsockopt(fd, SOL_IP, IPV6_DROP_MEMBERSHIP, &m6, sizeof(m6)); setsockopt(fd, SOL_IP, IPV6_ADD_MEMBERSHIP, &m6, 4); setsockopt(fd, SOL_IP, IPV6_DROP_MEMBERSHIP, &m6, 4); return 0; } And prints the following output when traced: setsockopt(3, SOL_IP, IP_ADD_MEMBERSHIP, "\340\0\0\2", 4) = -1 EINVAL (Invalid argument) setsockopt(3, SOL_IP, IP_DROP_MEMBERSHIP, "\340\0\0\2", 4) = -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_IP, IPV6_ADD_MEMBERSHIP, {ipv6mr_multiaddr=inet_addr("ff01::c"), ipv6mr_interface=42}, 20) = 0 setsockopt(3, SOL_IP, IPV6_DROP_MEMBERSHIP, {ipv6mr_multiaddr=inet_addr("ff01::c"), ipv6mr_interface=42}, 20) = -1 EINVAL (Invalid argument) setsockopt(3, SOL_IP, IPV6_ADD_MEMBERSHIP, "\377\1\0\0", 4) = 0 setsockopt(3, SOL_IP, IPV6_DROP_MEMBERSHIP, "\377\1\0\0", 4) = -1 EINVAL (Invalid argument) * net.c (sys_setsockopt): decode IP_ADD_MEMBERSHIP, IP_DROP_MEMBERSHIP, IPV6_ADD_MEMBERSHIP and IPV6_DROP_MEMBERSHIP arguments. Signed-off-by: Ben Noordhuis <in...@bn...> --- net.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/net.c b/net.c index a466efe..9cdd44e 100644 --- a/net.c +++ b/net.c @@ -2301,6 +2301,49 @@ static void printicmpfilter(struct tcb *tcp, long addr) } #endif /* ICMP_FILTER */ +#if defined(IP_ADD_MEMBERSHIP) || defined(IP_DROP_MEMBERSHIP) +static void printmreq(struct tcb *tcp, long addr, int len) +{ + struct ip_mreq mreq; + if (len == sizeof(mreq) && umove(tcp, addr, &mreq) == 0) { + tprintf("{imr_multiaddr=inet_addr(\"%s\"),", + inet_ntoa(mreq.imr_multiaddr)); + tprintf(" imr_interface=inet_addr(\"%s\")}", + inet_ntoa(mreq.imr_interface)); + } + else { + printstr(tcp, addr, len); + } +} +#endif /* defined(IP_ADD_MEMBERSHIP) || defined(IP_DROP_MEMBERSHIP) */ + +#if defined(IPV6_ADD_MEMBERSHIP) || defined(IPV6_DROP_MEMBERSHIP) +static void printmreq6(struct tcb *tcp, long addr, int len) +{ +#if HAVE_INET_NTOP + struct ipv6_mreq mreq; + const struct in6_addr *in6; + char text[INET6_ADDRSTRLEN]; + + if (len != sizeof(mreq)) + goto fail; + + if (umove(tcp, addr, &mreq) < 0) + goto fail; + + in6 = &mreq.ipv6mr_multiaddr; + if (inet_ntop(AF_INET6, in6, text, sizeof(text)) != text) + goto fail; + + tprintf("{ipv6mr_multiaddr=inet_ntop(\"%s\"), ipv6mr_interface=%d}", + text, mreq.ipv6mr_interface); + return; +fail: +#endif /* HAVE_INET_NTOP */ + printstr(tcp, addr, len); +} +#endif /* defined(IPV6_ADD_MEMBERSHIP) || defined(IPV6_DROP_MEMBERSHIP) */ + static int printsockopt(struct tcb *tcp, int level, int name, long addr, int len) { @@ -2327,7 +2370,34 @@ printsockopt(struct tcb *tcp, int level, int name, long addr, int len) break; #ifdef SOL_IP case SOL_IP: - printxval(sockipoptions, name, "IP_???"); + switch (name) { +#ifdef IP_ADD_MEMBERSHIP + case IP_ADD_MEMBERSHIP: + tprints("IP_ADD_MEMBERSHIP, "); + printmreq(tcp, addr, len); + return 0; +#endif +#ifdef IP_DROP_MEMBERSHIP + case IP_DROP_MEMBERSHIP: + tprints("IP_DROP_MEMBERSHIP, "); + printmreq(tcp, addr, len); + return 0; +#endif +#ifdef IPV6_ADD_MEMBERSHIP + case IPV6_ADD_MEMBERSHIP: + tprints("IPV6_ADD_MEMBERSHIP, "); + printmreq6(tcp, addr, len); + return 0; +#endif +#ifdef IPV6_DROP_MEMBERSHIP + case IPV6_DROP_MEMBERSHIP: + tprints("IPV6_DROP_MEMBERSHIP, "); + printmreq6(tcp, addr, len); + return 0; +#endif + default: + printxval(sockipoptions, name, "IP_???"); + } break; #endif #ifdef SOL_IPV6 -- 1.8.5.3 |
From: Philippe O. <pom...@ne...> - 2014-03-18 13:48:51
|
On Mon, Mar 17, 2014 at 7:24 PM, Ben Noordhuis <in...@bn...> 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 = 42; > setsockopt(fd, SOL_IP, IP_ADD_MEMBERSHIP, &m4, 4); > setsockopt(fd, SOL_IP, IP_DROP_MEMBERSHIP, &m4, 4); > setsockopt(fd, SOL_IP, IP_ADD_MEMBERSHIP, &m4, sizeof(m4)); > setsockopt(fd, SOL_IP, IP_DROP_MEMBERSHIP, &m4, sizeof(m4)); > setsockopt(fd, SOL_IP, IPV6_ADD_MEMBERSHIP, &m6, sizeof(m6)); > setsockopt(fd, SOL_IP, IPV6_DROP_MEMBERSHIP, &m6, sizeof(m6)); > setsockopt(fd, SOL_IP, IPV6_ADD_MEMBERSHIP, &m6, 4); > setsockopt(fd, SOL_IP, IPV6_DROP_MEMBERSHIP, &m6, 4); > return 0; > } > > And prints the following output when traced: > > setsockopt(3, SOL_IP, IP_ADD_MEMBERSHIP, "\340\0\0\2", 4) > = -1 EINVAL (Invalid argument) > setsockopt(3, SOL_IP, IP_DROP_MEMBERSHIP, "\340\0\0\2", 4) > = -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_IP, IPV6_ADD_MEMBERSHIP, > {ipv6mr_multiaddr=inet_addr("ff01::c"), ipv6mr_interface=42}, 20) > = 0 > setsockopt(3, SOL_IP, IPV6_DROP_MEMBERSHIP, > {ipv6mr_multiaddr=inet_addr("ff01::c"), ipv6mr_interface=42}, 20) > = -1 EINVAL (Invalid argument) > setsockopt(3, SOL_IP, IPV6_ADD_MEMBERSHIP, "\377\1\0\0", 4) = 0 > setsockopt(3, SOL_IP, IPV6_DROP_MEMBERSHIP, "\377\1\0\0", 4) > = -1 EINVAL (Invalid argument) Excellent, really thanks mucho for providing a test. I will let Dmitry review and comment on the patch proper. On my side I am interested in the test and I might try to make this part of the make check. Time to beef up a notch strace's test framework! -- Philippe Ombredanne |
From: Dmitry V. L. <ld...@al...> - 2014-03-19 01:19:46
|
On Mon, Mar 17, 2014 at 07:24:00PM +0100, Ben Noordhuis wrote: > * net.c (sys_setsockopt): decode IP_ADD_MEMBERSHIP, IP_DROP_MEMBERSHIP, > IPV6_ADD_MEMBERSHIP and IPV6_DROP_MEMBERSHIP arguments. Thanks. > --- a/net.c > +++ b/net.c > @@ -2301,6 +2301,49 @@ static void printicmpfilter(struct tcb *tcp, long addr) > } > #endif /* ICMP_FILTER */ > > +#if defined(IP_ADD_MEMBERSHIP) || defined(IP_DROP_MEMBERSHIP) Is it really possible to have only one of these constants defined? > +static void printmreq(struct tcb *tcp, long addr, int len) > +{ > + struct ip_mreq mreq; We usually add an empty line here. > + if (len == sizeof(mreq) && umove(tcp, addr, &mreq) == 0) { > + tprintf("{imr_multiaddr=inet_addr(\"%s\"),", > + inet_ntoa(mreq.imr_multiaddr)); > + tprintf(" imr_interface=inet_addr(\"%s\")}", > + inet_ntoa(mreq.imr_interface)); > + } > + else { > + printstr(tcp, addr, len); > + } > +} > +#endif /* defined(IP_ADD_MEMBERSHIP) || defined(IP_DROP_MEMBERSHIP) */ > + > +#if defined(IPV6_ADD_MEMBERSHIP) || defined(IPV6_DROP_MEMBERSHIP) > +static void printmreq6(struct tcb *tcp, long addr, int len) > +{ > +#if HAVE_INET_NTOP > + struct ipv6_mreq mreq; > + const struct in6_addr *in6; > + char text[INET6_ADDRSTRLEN]; > + > + if (len != sizeof(mreq)) > + goto fail; > + > + if (umove(tcp, addr, &mreq) < 0) > + goto fail; > + > + in6 = &mreq.ipv6mr_multiaddr; > + if (inet_ntop(AF_INET6, in6, text, sizeof(text)) != text) > + goto fail; > + > + tprintf("{ipv6mr_multiaddr=inet_ntop(\"%s\"), ipv6mr_interface=%d}", > + text, mreq.ipv6mr_interface); > + return; > +fail: The same code written without "goto" would be more readable: if (len == sizeof(mreq) && umove(tcp, addr, &mreq) == 0 && inet_ntop(AF_INET6, &mreq.ipv6mr_multiaddr, text, sizeof(text))) { tprintf(...); return; } > +#endif /* HAVE_INET_NTOP */ > + printstr(tcp, addr, len); > +} > +#endif /* defined(IPV6_ADD_MEMBERSHIP) || defined(IPV6_DROP_MEMBERSHIP) */ > + > static int > printsockopt(struct tcb *tcp, int level, int name, long addr, int len) > { > @@ -2327,7 +2370,34 @@ printsockopt(struct tcb *tcp, int level, int name, long addr, int len) > break; > #ifdef SOL_IP > case SOL_IP: > - printxval(sockipoptions, name, "IP_???"); > + switch (name) { > +#ifdef IP_ADD_MEMBERSHIP > + case IP_ADD_MEMBERSHIP: > + tprints("IP_ADD_MEMBERSHIP, "); > + printmreq(tcp, addr, len); > + return 0; > +#endif > +#ifdef IP_DROP_MEMBERSHIP > + case IP_DROP_MEMBERSHIP: > + tprints("IP_DROP_MEMBERSHIP, "); > + printmreq(tcp, addr, len); > + return 0; > +#endif > +#ifdef IPV6_ADD_MEMBERSHIP > + case IPV6_ADD_MEMBERSHIP: > + tprints("IPV6_ADD_MEMBERSHIP, "); > + printmreq6(tcp, addr, len); > + return 0; > +#endif > +#ifdef IPV6_DROP_MEMBERSHIP > + case IPV6_DROP_MEMBERSHIP: > + tprints("IPV6_DROP_MEMBERSHIP, "); > + printmreq6(tcp, addr, len); > + return 0; > +#endif > + default: > + printxval(sockipoptions, name, "IP_???"); > + } Could it be coded in a more compact way? e.g. if ADD_MEMBERSHIP and DROP_MEMBERSHIP constants are either defined or undefined altogether: printxval(sockipoptions, name, "IP_???"); switch (name) { #ifdef IP_ADD_MEMBERSHIP case IP_ADD_MEMBERSHIP: case IP_DROP_MEMBERSHIP: tprints(", "); printmreq(tcp, addr, len); return 0; #endif #ifdef IPV6_ADD_MEMBERSHIP case IPV6_ADD_MEMBERSHIP: case IPV6_DROP_MEMBERSHIP: tprints(", "); printmreq6(tcp, addr, len); return 0; #endif default: break; } > break; > #endif > #ifdef SOL_IPV6 -- ldv |
From: Ben N. <in...@bn...> - 2015-02-05 18:52:34
|
* net.c (printsock): DRY if_indextoname() logic. Signed-off-by: Ben Noordhuis <in...@bn...> --- net.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/net.c b/net.c index f55c6af..ca108e1 100644 --- a/net.c +++ b/net.c @@ -200,6 +200,9 @@ # include "xlat/af_packet_types.h" #endif +static void +print_interface(unsigned int index); + void printsock(struct tcb *tcp, long addr, int addrlen) { @@ -279,29 +282,14 @@ printsock(struct tcb *tcp, long addr, int addrlen) ntohs(addrbuf.sa6.sin6_port), string_addr, addrbuf.sa6.sin6_flowinfo); #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID - { -#if defined(HAVE_IF_INDEXTONAME) && defined(IN6_IS_ADDR_LINKLOCAL) && defined(IN6_IS_ADDR_MC_LINKLOCAL) - int numericscope = 0; - if (IN6_IS_ADDR_LINKLOCAL(&addrbuf.sa6.sin6_addr) - || IN6_IS_ADDR_MC_LINKLOCAL(&addrbuf.sa6.sin6_addr)) { - char scopebuf[IFNAMSIZ + 1]; - - if (if_indextoname(addrbuf.sa6.sin6_scope_id, scopebuf) == NULL) - numericscope++; - else { - tprints(", sin6_scope_id=if_nametoindex("); - print_quoted_string(scopebuf, - sizeof(scopebuf), - QUOTE_0_TERMINATED); - tprints(")"); - } - } else - numericscope++; - - if (numericscope) -#endif - tprintf(", sin6_scope_id=%u", addrbuf.sa6.sin6_scope_id); - } + tprints(", sin6_scope_id="); +#if defined(IN6_IS_ADDR_LINKLOCAL) && defined(IN6_IS_ADDR_MC_LINKLOCAL) + if (IN6_IS_ADDR_LINKLOCAL(&addrbuf.sa6.sin6_addr) + || IN6_IS_ADDR_MC_LINKLOCAL(&addrbuf.sa6.sin6_addr)) + print_interface(addrbuf.sa6.sin6_scope_id); + else +#endif + tprintf("%u", addrbuf.sa6.sin6_scope_id); #endif break; #endif -- 2.1.0 |
From: Ben N. <in...@bn...> - 2015-02-05 18:54:28
|
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) * net.c (sys_setsockopt): decode IP_ADD_MEMBERSHIP, IP_DROP_MEMBERSHIP, IPV6_ADD_MEMBERSHIP and IPV6_DROP_MEMBERSHIP arguments. Signed-off-by: Ben Noordhuis <in...@bn...> --- 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) { + tprints("if_nametoindex("); + print_quoted_string(buf, sizeof(buf), QUOTE_0_TERMINATED); + tprints(")"); + return; + } +#endif + tprintf("%u", index); +} + +#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); + } +} +#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; + + in6 = &mreq.ipv6mr_multiaddr; + if (inet_ntop(AF_INET6, in6, address, sizeof(address)) != address) + goto fail; + + tprints("{ipv6mr_multiaddr=inet_pton("); + print_quoted_string(address, sizeof(address), QUOTE_0_TERMINATED); + tprints("), ipv6mr_interface="); + print_interface(mreq.ipv6mr_interface); + tprints("}"); + return; + +fail: +#endif /* HAVE_INET_NTOP */ + printstr(tcp, addr, len); +} +#endif /* IPV6_ADD_MEMBERSHIP */ + #ifdef MCAST_JOIN_GROUP static void print_group_req(struct tcb *tcp, long addr, int len) @@ -1438,6 +1505,12 @@ print_setsockopt(struct tcb *tcp, int level, int name, long addr, int len) case SOL_IP: switch (name) { +#ifdef IP_ADD_MEMBERSHIP + case IP_ADD_MEMBERSHIP: + case IP_DROP_MEMBERSHIP: + print_mreq(tcp, addr, len); + goto done; +#endif /* IP_ADD_MEMBERSHIP */ #ifdef MCAST_JOIN_GROUP case MCAST_JOIN_GROUP: case MCAST_LEAVE_GROUP: @@ -1445,6 +1518,18 @@ print_setsockopt(struct tcb *tcp, int level, int name, long addr, int len) goto done; #endif /* MCAST_JOIN_GROUP */ } + break; + + case SOL_IPV6: + switch (name) { +#ifdef IPV6_ADD_MEMBERSHIP + case IPV6_ADD_MEMBERSHIP: + case IPV6_DROP_MEMBERSHIP: + print_mreq6(tcp, addr, len); + goto done; +#endif /* IPV6_ADD_MEMBERSHIP */ + } + break; case SOL_PACKET: switch (name) { -- 2.1.0 |
From: Dmitry V. L. <ld...@al...> - 2015-02-06 23:55:24
|
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 <in...@bn...> > --- > 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 |
From: Ben N. <in...@bn...> - 2015-02-07 17:35:46
|
On Sat, Feb 7, 2015 at 12:55 AM, Dmitry V. Levin <ld...@al...> wrote: > On Thu, Feb 05, 2015 at 07:28:45PM +0100, Ben Noordhuis wrote: >> +#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. Printing the raw data helps troubleshooting more than printing a memory address does, IMO. printstr() is also what print_getsockopt() and print_setsockopt() seem to fall back to. Let me know what you think is preferable. I agree that there is no point calling printstr() when umove() fails, I'll update that. |
From: Dmitry V. L. <ld...@al...> - 2015-02-07 17:17:54
|
On Sat, Feb 07, 2015 at 06:05:56PM +0100, Ben Noordhuis wrote: > On Sat, Feb 7, 2015 at 12:55 AM, Dmitry V. Levin <ld...@al...> wrote: > > On Thu, Feb 05, 2015 at 07:28:45PM +0100, Ben Noordhuis wrote: > >> +#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. > > Printing the raw data helps troubleshooting more than printing a > memory address does, IMO. printstr() is also what print_getsockopt() > and print_setsockopt() seem to fall back to. Let me know what you > think is preferable. Agreed, lets fall back to printstr() in case of length mismatch. > I agree that there is no point calling printstr() when umove() fails, > I'll update that. -- ldv |
From: Dmitry V. L. <ld...@al...> - 2015-05-26 14:26:37
|
On Sat, Feb 07, 2015 at 08:17:46PM +0300, Dmitry V. Levin wrote: > On Sat, Feb 07, 2015 at 06:05:56PM +0100, Ben Noordhuis wrote: > > On Sat, Feb 7, 2015 at 12:55 AM, Dmitry V. Levin wrote: > > > On Thu, Feb 05, 2015 at 07:28:45PM +0100, Ben Noordhuis wrote: > > >> +#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. > > > > Printing the raw data helps troubleshooting more than printing a > > memory address does, IMO. printstr() is also what print_getsockopt() > > and print_setsockopt() seem to fall back to. Let me know what you > > think is preferable. > > Agreed, lets fall back to printstr() in case of length mismatch. > > > I agree that there is no point calling printstr() when umove() fails, > > I'll update that. Are you still working on that update? -- ldv |
From: Ben N. <in...@bn...> - 2015-05-26 15:47:46
|
On Tue, May 26, 2015 at 4:26 PM, Dmitry V. Levin <ld...@al...> wrote: > On Sat, Feb 07, 2015 at 08:17:46PM +0300, Dmitry V. Levin wrote: >> On Sat, Feb 07, 2015 at 06:05:56PM +0100, Ben Noordhuis wrote: >> > On Sat, Feb 7, 2015 at 12:55 AM, Dmitry V. Levin wrote: >> > > On Thu, Feb 05, 2015 at 07:28:45PM +0100, Ben Noordhuis wrote: >> > >> +#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. >> > >> > Printing the raw data helps troubleshooting more than printing a >> > memory address does, IMO. printstr() is also what print_getsockopt() >> > and print_setsockopt() seem to fall back to. Let me know what you >> > think is preferable. >> >> Agreed, lets fall back to printstr() in case of length mismatch. >> >> > I agree that there is no point calling printstr() when umove() fails, >> > I'll update that. > > Are you still working on that update? I have it on my todo list but the days don't have enough hours in them. If someone else wants to pick up the patches, s/he has my blessing; no attribution needed. |