[PATCH v2 4/6] socketutils: build a xlat table dynamically for decoding "family" field of Netlink GENERIC protocol

[Dmitry V. Levin]

On Sun, Jun 11, 2017 at 04:42:30PM +0900, Masatake YAMATO wrote:
> * socketutils.c (genl_families_xlat): exported function for building
> the xlat table. Include linux/genetlink.h.
> 
> * defs.h (genl_families_xlat): add the declaration.
> (genl_send_dump_families, genl_parse_families_response):
> helper functions.
> 
> Signed-off-by: Masatake YAMATO <yamato at redhat.com>
> ---
>  defs.h        |  3 ++
>  socketutils.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/defs.h b/defs.h
> index 05958fd..eca8058 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -1025,4 +1025,7 @@ struct dyxlat *dyxlat_new(int allocation);
>  void dyxlat_delete(struct dyxlat *dyxlat);
>  struct xlat *dyxlat_get(struct dyxlat *dyxlat);
>  void dyxlat_may_add_pair(struct dyxlat *dyxlat, uint64_t val, const char *str);
> +
> +struct xlat *genl_families_xlat(void);
> +
>  #endif /* !STRACE_DEFS_H */
> diff --git a/socketutils.c b/socketutils.c
> index 1e83f87..883fe8b 100644
> --- a/socketutils.c
> +++ b/socketutils.c
> @@ -37,6 +37,7 @@
>  #include <linux/unix_diag.h>
>  #include <linux/netlink_diag.h>
>  #include <linux/rtnetlink.h>
> +#include <linux/genetlink.h>

<linux/genetlink.h> was introduced in linux 2.6.15, should we add
a configure check?

>  
>  #include <sys/un.h>
>  #ifndef UNIX_PATH_MAX
> @@ -541,3 +542,100 @@ print_sockaddr_by_inode(struct tcb *const tcp, const int fd,
>  	return print_sockaddr_by_inode_cached(inode) ? true :
>  		print_sockaddr_by_inode_uncached(inode, getfdproto(tcp, fd));
>  }
> +
> +/*
> + * Managing the cache for decoding communications of Netlink GENERIC protocol
> + *
> + * As name shown Netlink GENERIC protocol is generic protocol. The
> + * numbers of msg types used in the protocol are not defined
> + * statically. Kernel defines them on demand.  So the xlat converted
> + * from header files doesn't help for decoding the protocol. Following
> + * codes are building xlat(dyxlat) at runtime.
> + */
> +static bool
> +genl_send_dump_families(const int fd)
> +{
> +	struct {
> +		const struct nlmsghdr nlh;
> +		struct genlmsghdr gnlh;
> +	} req = {
> +		.nlh = {
> +			.nlmsg_len = sizeof(req),
> +			.nlmsg_type = GENL_ID_CTRL,
> +			.nlmsg_flags = NLM_F_DUMP|NLM_F_REQUEST|NLM_F_ACK,

Is NLM_F_ACK needed here?

> +		},
> +		.gnlh = {
> +			.cmd = CTRL_CMD_GETFAMILY,
> +		}
> +	};
> +	return send_query(fd, &req, sizeof(req));
> +}
> +
> +static int
> +genl_parse_families_response(const void *const data,
> +			     const int data_len, const unsigned long inode,
> +			     void *user_data)
> +{
> +	struct dyxlat *dyxlat = user_data;
> +	const struct genlmsghdr * const gnlh = data;
> +	struct rtattr *attr;
> +	int rta_len = data_len - NLMSG_LENGTH(sizeof(*gnlh));
> +
> +	char *name = NULL;
> +	uint16_t id = 0;
> +	bool id_set = false;
> +
> +	if (rta_len < 0)
> +		return -1;
> +	if (gnlh->cmd != CTRL_CMD_NEWFAMILY)
> +		return -1;
> +	if (gnlh->version != 2)
> +		return -1;
> +
> +	for (attr = (struct rtattr *) (gnlh + 1);
> +	     RTA_OK(attr, rta_len);
> +	     attr = RTA_NEXT(attr, rta_len)) {
> +		switch (attr->rta_type) {
> +		case CTRL_ATTR_FAMILY_NAME:
> +			name = xstrdup(RTA_DATA(attr));

This should be coded more defensively, like unix_parse_response,
we shouldn't assume that RTA_DATA(attr) is NUL-terminated, and that
CTRL_ATTR_FAMILY_NAME attribute is present only once per message.

> +			break;
> +		case CTRL_ATTR_FAMILY_ID:
> +			if (RTA_PAYLOAD(attr) >= 2) {
> +				id = *(uint16_t *)(RTA_DATA(attr));
> +				id_set = true;
> +			}

Let's just save the pointer here, this way we won't need id_set.

> +			break;
> +		}
> +		if (name && id_set) {
> +			dyxlat_may_add_pair(dyxlat, id, name);
> +			id_set = false;
> +			free(name);
> +			name = NULL;
> +		}

I think we shouldn't leak memory if CTRL_ATTR_FAMILY_NAME attribute
is present without CTRL_ATTR_FAMILY_ID.


-- 
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/20170613/885ca542/attachment.bin>