Menu
▾
▴
Re: [PATCH 1/2] Decode setsockopt() multicast arguments
|
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
|