[PATCH v4 01/10] netlink: introduce a dummy netlink attributes parser

Dmitry V. Levin ldv at altlinux.org
Wed Jun 21 13:01:19 UTC 2017


On Tue, Jun 20, 2017 at 05:23:46PM +0800, JingPiao Chen wrote:
> * linux/unix_diag.h (UNIX_DIAG_*): New enum.
> * nlattr.c: New file.
> * nlattr.h: Likewise.
> * Makefile.am (strace_SOURCES): Add them.
> * netlink_sock_diag.c: Include "nlattr.h" and "xlat/unix_diag_attrs.h".
> (decode_unix_diag_msg): Use decode_nlattr.
> * xlat/unix_diag_attrs.in: New file.
> 
> co-authored-by: Fabien Siron <fabien.siron at epita.fr>

Co-authored-by starts with a capital letter.

The change is fine, but there are few portability issues
that have to be addressed.

> ---
>  Makefile.am             |   2 +
>  linux/unix_diag.h       |  11 +++-
>  netlink_sock_diag.c     |  13 ++++-
>  nlattr.c                | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
>  nlattr.h                |  32 ++++++++++++
>  xlat/unix_diag_attrs.in |   8 +++
>  6 files changed, 194 insertions(+), 3 deletions(-)
>  create mode 100644 nlattr.c
>  create mode 100644 nlattr.h
>  create mode 100644 xlat/unix_diag_attrs.in
> 
> diff --git a/Makefile.am b/Makefile.am
> index f9e78ed..50bf832 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -174,6 +174,8 @@ strace_SOURCES =	\
>  	net.c		\
>  	netlink.c       \
>  	netlink_sock_diag.c \
> +	nlattr.c	\
> +	nlattr.h	\
>  	nsfs.c          \
>  	nsfs.h          \
>  	nsig.h		\
> diff --git a/linux/unix_diag.h b/linux/unix_diag.h
> index a6b62ba..3c9d99f 100644
> --- a/linux/unix_diag.h
> +++ b/linux/unix_diag.h
> @@ -27,7 +27,14 @@ struct unix_diag_msg {
>  	uint32_t udiag_cookie[2];
>  };
>  
> -#define UNIX_DIAG_NAME 0
> -#define UNIX_DIAG_PEER 2
> +enum {
> +	UNIX_DIAG_NAME,
> +	UNIX_DIAG_VFS,
> +	UNIX_DIAG_PEER,
> +	UNIX_DIAG_ICONS,
> +	UNIX_DIAG_RQLEN,
> +	UNIX_DIAG_MEMINFO,
> +	UNIX_DIAG_SHUTDOWN,
> +};
>  
>  #endif /* !STRACE_LINUX_UNIX_DIAG_H */
> diff --git a/netlink_sock_diag.c b/netlink_sock_diag.c
> index 8dbfd07..f92253e 100644
> --- a/netlink_sock_diag.c
> +++ b/netlink_sock_diag.c
> @@ -28,6 +28,7 @@
>   */
>  
>  #include "defs.h"
> +#include "nlattr.h"
>  
>  #include <sys/socket.h>
>  #include <arpa/inet.h>
> @@ -55,6 +56,7 @@
>  # include "xlat/smc_states.h"
>  #endif
>  
> +#include "xlat/unix_diag_attrs.h"
>  #include "xlat/unix_diag_show.h"
>  
>  static void
> @@ -113,7 +115,8 @@ decode_unix_diag_msg(struct tcb *const tcp,
>  		     const kernel_ulong_t len)
>  {
>  	struct unix_diag_msg msg = { .udiag_family = family };
> -	const size_t offset = sizeof(msg.udiag_family);
> +	size_t offset = sizeof(msg.udiag_family);
> +	bool decode_nla = false;
>  
>  	tprints("{udiag_family=");
>  	printxval(addrfams, msg.udiag_family, "AF_???");
> @@ -131,10 +134,18 @@ decode_unix_diag_msg(struct tcb *const tcp,
>  				", udiag_cookie=[%" PRIu32 ", %" PRIu32 "]",
>  				msg.udiag_ino,
>  				msg.udiag_cookie[0], msg.udiag_cookie[1]);
> +			decode_nla = true;
>  		}
>  	} else
>  		tprints("...");
>  	tprints("}");
> +
> +	offset = NLMSG_ALIGN(sizeof(msg));
> +	if (decode_nla && len > offset) {
> +		tprints(", ");
> +		decode_nlattr(tcp, addr + offset, len - offset,
> +			      unix_diag_attrs, "UNIX_DIAG_???");
> +	}
>  }
>  
>  static void
> diff --git a/nlattr.c b/nlattr.c
> new file mode 100644
> index 0000000..eaf7e39
> --- /dev/null
> +++ b/nlattr.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (c) 2016 Fabien Siron <fabien.siron at epita.fr>
> + * Copyright (c) 2017 JingPiao Chen <chenjingpiao at gmail.com>
> + * Copyright (c) 2016-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 "defs.h"
> +
> +#include <linux/netlink.h>

<sys/socket.h> has to be included before <linux/netlink.h> because
the latter used to be incomplete in older kernel headers.
Failing to do this leads to the following build failure
on rhel6 and older systems:

In file included from nlattr.c:32:
/usr/include/linux/netlink.h:35: error: expected specifier-qualifier-list before 'sa_family_t'

> +
> +static bool
> +fetch_nlattr(struct tcb *const tcp, struct nlattr *const nlattr,
> +	     const kernel_ulong_t addr, const kernel_ulong_t len)
> +{
> +	if (len < sizeof(struct nlattr)) {
> +		printstrn(tcp, addr, len);
> +		return false;
> +	}
> +
> +	if (umove_or_printaddr(tcp, addr, nlattr))
> +		return false;
> +
> +	return true;
> +}
> +
> +static void
> +print_nlattr(const struct nlattr *const nla,
> +	     const struct xlat *const table,
> +	     const char *const dflt)
> +{
> +	tprintf("{nla_len=%u, nla_type=", nla->nla_len);
> +	if (nla->nla_type & NLA_F_NESTED)
> +		tprints("NLA_F_NESTED|");
> +	if (nla->nla_type & NLA_F_NET_BYTEORDER)
> +		tprints("NLA_F_NET_BYTEORDER|");
> +	printxval(table, nla->nla_type & NLA_TYPE_MASK, dflt);
> +	tprints("}");

NLA_F_NESTED, NLA_F_NET_BYTEORDER, and NLA_TYPE_MASK were introduced
by kernel commit v2.6.24-rc1~1454^2~685 and therefore is not available
in older systems like rhel5:

nlattr.c: In function 'print_nlattr':
nlattr.c:56: error: 'NLA_F_NESTED' undeclared (first use in this function)
nlattr.c:56: error: (Each undeclared identifier is reported only once
nlattr.c:56: error: for each function it appears in.)
nlattr.c:58: error: 'NLA_F_NET_BYTEORDER' undeclared (first use in this function)
nlattr.c:60: error: 'NLA_TYPE_MASK' undeclared (first use in this function)

> +}
> +
> +static void
> +decode_nlattr_with_data(struct tcb *tcp,
> +			const struct nlattr *const nla,
> +			kernel_ulong_t addr,
> +			kernel_ulong_t len,
> +			const struct xlat *const table,
> +			const char *const dflt)
> +{
> +	unsigned int nla_len = nla->nla_len > len ? len : nla->nla_len;

It's not important at all, but I'd add "const" qualifier here.

> +
> +	if (nla_len > NLA_HDRLEN)
> +		tprints("{");
> +
> +	print_nlattr(nla, table, dflt);
> +
> +	if (nla_len > NLA_HDRLEN) {
> +		tprints(", ");
> +		printstrn(tcp, addr + NLA_HDRLEN, nla_len - NLA_HDRLEN);
> +		tprints("}");
> +	}

NLA_HDRLEN was introduced by kernel commit v2.6.15-rc1~53^2~5 and
therefore is not available in older systems, although those systems
are rare nowadays.

As we have to deal with NLA_TYPE_MASK issue, we can handle this case
as well.

So far I have no better ideas than to introduce a local header,
e.g. netlink.h, containing, besides an include guard, something like this:

#include <sys/socket.h>
#include <linux/netlink.h>

#undef NLMSG_HDRLEN
#define NLMSG_HDRLEN ((unsigned int) NLMSG_ALIGN(sizeof(struct nlmsghdr)))

#ifndef NLA_ALIGN
# define NLA_ALIGN(len) (((len) + 3) & ~3)
#endif

#undef NLA_HDRLEN
#define NLA_HDRLEN ((unsigned int) NLA_ALIGN(sizeof(struct nlattr)))

#ifndef NLA_TYPE_MASK
# define NLA_F_NESTED		(1 << 15)
# define NLA_F_NET_BYTEORDER	(1 << 14)
# define NLA_TYPE_MASK		~(NLA_F_NESTED | NLA_F_NET_BYTEORDER)
#endif

Then "netlink.h" can be included everywhere instead of <linux/netlink.h>.

> +}
> +
> +void
> +decode_nlattr(struct tcb *const tcp,
> +	      kernel_ulong_t addr,
> +	      kernel_ulong_t len,
> +	      const struct xlat *const table,
> +	      const char *const dflt)
> +{
> +	struct nlattr nla;
> +	bool print_array = false;
> +	unsigned int elt;
> +
> +	for (elt = 0; fetch_nlattr(tcp, &nla, addr, len); elt++) {
> +		if (abbrev(tcp) && elt == max_strlen) {
> +			tprints("...");
> +			break;
> +		}
> +
> +		unsigned long nla_len = NLA_ALIGN(nla.nla_len);

It's not important at all, but I'd add "const" qualifier here.

> +		kernel_ulong_t next_addr = 0;
> +		kernel_ulong_t next_len = 0;
> +
> +		if (nla.nla_len >= NLA_HDRLEN) {
> +			next_len = (len >= nla_len) ? len - nla_len : 0;
> +
> +			if (next_len && addr + nla_len > addr)
> +				next_addr = addr + nla_len;
> +		}
> +
> +		if (!print_array && next_addr) {
> +			tprints("[");
> +			print_array = true;
> +		}
> +
> +		decode_nlattr_with_data(tcp, &nla, addr, len, table, dflt);
> +
> +		if (!next_addr)
> +			break;
> +
> +		tprints(", ");
> +		addr = next_addr;
> +		len = next_len;
> +	}
> +
> +	if (print_array) {
> +		tprints("]");
> +	}
> +}
> diff --git a/nlattr.h b/nlattr.h
> new file mode 100644
> index 0000000..6b94fc1
> --- /dev/null
> +++ b/nlattr.h
> @@ -0,0 +1,32 @@
> +/*
> + * Copyright (c) 2016 Fabien Siron <fabien.siron at epita.fr>
> + * Copyright (c) 2017 JingPiao Chen <chenjingpiao at gmail.com>
> + * Copyright (c) 2016-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.
> + */
> +
> +extern void
> +decode_nlattr(struct tcb *, kernel_ulong_t addr, kernel_ulong_t len,
> +	      const struct xlat *, const char *);

Let's add a regular include guard to this header, too.

> diff --git a/xlat/unix_diag_attrs.in b/xlat/unix_diag_attrs.in
> new file mode 100644
> index 0000000..4c3d9b2
> --- /dev/null
> +++ b/xlat/unix_diag_attrs.in
> @@ -0,0 +1,8 @@
> +#unconditional
> +UNIX_DIAG_NAME
> +UNIX_DIAG_VFS
> +UNIX_DIAG_PEER
> +UNIX_DIAG_ICONS
> +UNIX_DIAG_RQLEN
> +UNIX_DIAG_MEMINFO
> +UNIX_DIAG_SHUTDOWN

-- 
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/20170621/755c7d95/attachment.bin>


More information about the Strace-devel mailing list