On Tue, Jun 13, 2017 at 05:26:43PM +0900, Masatake YAMATO wrote: > * configure.ac: Verify existence of linux/genetlink.h. > > * 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. > > Changes in v3 (all suggested by ldv): > > * Check whether linux/genetlink.h is available or not. > > * Don't add NLM_F_ACK to nlmsg_type when dumping genl families. > > * Use xstrndup to extract family name from netlink data. > xstrndup can limits the length of data copied and puts > nul char at the end of buffer. > > * Free the buffer for storing family name before overwriting it. > > * Remove id_set local variable. id is now a pointer. So NULL > check of id can be used to know whether the value is assigned or not. > > * Free the buffer for storing family name even if it is not stored > to dyxlat. > > Signed-off-by: Masatake YAMATO <yam...@redhat.com> > --- > configure.ac | 1 + > defs.h | 2 ++ > socketutils.c | 110 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 113 insertions(+) > > diff --git a/configure.ac b/configure.ac > index dc49d39..0974743 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -366,6 +366,7 @@ AC_CHECK_HEADERS(m4_normalize([ > linux/falloc.h > linux/fiemap.h > linux/filter.h > + linux/genetlink.h > linux/hiddev.h > linux/ip_vs.h > linux/ipc.h > diff --git a/defs.h b/defs.h > index 063394e..f50b474 100644 > --- a/defs.h > +++ b/defs.h > @@ -511,6 +511,8 @@ void dyxlat_free(struct dyxlat *dyxlat); > struct xlat *dyxlat_get(struct dyxlat *dyxlat); > void dyxlat_add_pair(struct dyxlat *dyxlat, uint64_t val, const char *str); > > +struct xlat *genl_families_xlat(void);
Let's change this prototype to
const struct xlat *genl_families_xlat(void);
> +
> extern unsigned long get_pagesize(void);
> extern int
> string_to_uint_ex(const char *str, char **endptr,
> diff --git a/socketutils.c b/socketutils.c
> index 6486e27..bc18430 100644
> --- a/socketutils.c
> +++ b/socketutils.c
> @@ -37,6 +37,9 @@
> #include <linux/unix_diag.h>
> #include <linux/netlink_diag.h>
> #include <linux/rtnetlink.h>
> +#if HAVE_LINUX_GENETLINK_H
> +#include <linux/genetlink.h>
> +#endif
>
> #include <sys/un.h>
> #ifndef UNIX_PATH_MAX
> @@ -541,3 +544,110 @@ 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));
> }
> +
> +#ifdef HAVE_LINUX_GENETLINK_H
> +/*
> + * 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,
> + },
> + .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 = NULL;
> +
> + 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:
> + if (name)
> + free(name);
What if we used the first occurrence instead?
> + name = xstrndup(RTA_DATA(attr), RTA_PAYLOAD(attr));
What if we just saved the pointer? I mean something like this:
char *name = NULL;
unsigned int name_len = 0;
...
case CTRL_ATTR_FAMILY_NAME:
if (!name) {
name = RTA_DATA(attr);
name_len = RTA_PAYLOAD(attr);
}
...
if (name && name_len && id)
dyxlat_add_pair(dyxlat, *id, name, name_len);
> + break;
> + case CTRL_ATTR_FAMILY_ID:
> + if (RTA_PAYLOAD(attr) >= 2)
Wouldn't it be better to use a strict check, e.g.
if (!id && RTA_PAYLOAD(attr) == sizeof(*id))
--
ldv
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
