[PATCH v3 02/16] tests: check decoding of netlink attribute

[Dmitry V. Levin]

On Mon, Jun 19, 2017 at 09:15:26AM +0800, JingPiao Chen wrote:
> * tests/netlink_attribute.c: New file.
> * tests/gen_tests.in (netlink_attribute): New entry.
> * tests/pure_executables.list: Add netlink_attribute.
> * tests/.gitignore: Likewise.

I suggest a shorter name for this test: nlattr.

> ---
>  tests/.gitignore            |   1 +
>  tests/gen_tests.in          |   1 +
>  tests/netlink_attribute.c   | 326 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/pure_executables.list |   1 +
>  4 files changed, 329 insertions(+)
>  create mode 100644 tests/netlink_attribute.c
> 
> diff --git a/tests/.gitignore b/tests/.gitignore
> index de9fccd..582792b 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -197,6 +197,7 @@ net-y-unix
>  net-yy-inet
>  net-yy-netlink
>  net-yy-unix
> +netlink_attribute
>  netlink_audit
>  netlink_inet_diag
>  netlink_netfilter
> diff --git a/tests/gen_tests.in b/tests/gen_tests.in
> index 24a0bfa..476e59b 100644
> --- a/tests/gen_tests.in
> +++ b/tests/gen_tests.in
> @@ -184,6 +184,7 @@ munlockall	-a13
>  nanosleep	-a20
>  net-icmp_filter	-e trace=getsockopt,setsockopt
>  net-sockaddr	-a24 -e trace=connect
> +netlink_attribute	+netlink_sock_diag.test
>  netlink_audit	+netlink_sock_diag.test
>  netlink_netfilter	+netlink_sock_diag.test
>  netlink_protocol	-e trace=sendto
> diff --git a/tests/netlink_attribute.c b/tests/netlink_attribute.c
> new file mode 100644
> index 0000000..61c75a0
> --- /dev/null
> +++ b/tests/netlink_attribute.c
> @@ -0,0 +1,326 @@
> +/*
> + * Check decoding of netlink attribute.
> + *
> + * Copyright (c) 2017 JingPiao Chen <chenjingpiao at gmail.com>
> + * Copyright (c) 2017 The strace developers.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + *    derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "tests.h"
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/socket.h>
> +#include <netinet/tcp.h>
> +#include <linux/netlink.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/sock_diag.h>
> +#include <linux/unix_diag.h>
> +
> +#if !defined NETLINK_SOCK_DIAG && defined NETLINK_INET_DIAG
> +# define NETLINK_SOCK_DIAG NETLINK_INET_DIAG
> +#endif
> +
> +static void
> +test_nlattr(const int fd)
> +{
> +	static const struct msg {
> +		struct nlmsghdr nlh;
> +		struct unix_diag_msg udm;
> +	} c_msg = {
> +		.nlh = {
> +			.nlmsg_len = sizeof(struct msg),
> +			.nlmsg_type = SOCK_DIAG_BY_FAMILY,
> +			.nlmsg_flags = NLM_F_DUMP
> +		},
> +		.udm = {
> +			.udiag_family = AF_UNIX,
> +			.udiag_type = SOCK_STREAM,
> +			.udiag_state = TCP_FIN_WAIT1
> +		}
> +	};
> +	struct msg *msg;
> +	struct nlattr *nla;
> +	unsigned int msg_len;
> +	long rc;
> +
> +	/* fetch fail: len < sizeof(struct nlattr) */
> +	msg_len = NLMSG_SPACE(sizeof(msg->udm)) + 2;
> +	msg = tail_memdup(&c_msg, msg_len);
> +	msg->nlh.nlmsg_len = msg_len;

Here the address of msg->nlh.nlmsg_len is not aligned on int's boundary,
assigning an integer to this address, as well as reading an integer from
this address, causes unaligned access.  On some architectures (like sparc)
unaligned access causes SIGBUS.

There is a simple method of avoiding this problem:
	memcpy(&msg->nlh.nlmsg_len, &msg_len, sizeof(msg_len));

> +	nla = (void *) msg + NLMSG_SPACE(sizeof(msg->udm));
> +	memcpy(nla, "12", 2);
> +	rc = sendto(fd, msg, msg->nlh.nlmsg_len, MSG_DONTWAIT, NULL, 0);
> +	printf("sendto(%d, {{len=%u, type=SOCK_DIAG_BY_FAMILY"
> +	       ", flags=NLM_F_DUMP, seq=0, pid=0}, {udiag_family=AF_UNIX"
> +	       ", udiag_type=SOCK_STREAM, udiag_state=TCP_FIN_WAIT1"
> +	       ", udiag_ino=0, udiag_cookie=[0, 0]}, \"12\"}, %u"
> +	       ", MSG_DONTWAIT, NULL, 0) = %s\n",
> +	       fd, msg->nlh.nlmsg_len, msg->nlh.nlmsg_len, sprintrc(rc));

msg_len is not just shorter than msg->nlh.nlmsg_len, it's also safe.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170620/90854741/attachment.bin>