[PATCH 1/2] Decode setsockopt() multicast arguments

Dmitry V. Levin ldv at altlinux.org
Fri Feb 6 23:55:16 UTC 2015


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 <info at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20150207/47963736/attachment.bin>


More information about the Strace-devel mailing list