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

Attachment: 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

Reply via email to