[PATCH v2 13/16] netlink: introduce nla_decoder_t to parse netlink attribute data

JingPiao Chen chenjingpiao at gmail.com
Sun Jun 18 08:43:47 UTC 2017


On Sun, Jun 18, 2017 at 11:35:10AM +0300, Dmitry V. Levin wrote:
> On Sun, Jun 18, 2017 at 01:18:40PM +0800, JingPiao Chen wrote:
> > * defs.h (decode_nlattr): Move ...
> > * nlattr.h: ... here, new file.
> > (decode_nlattr): Add decoders, size and opaque_data argument.
> > * Makefile.am (strace_SOURCES): Add it.
> > * nlattr.c: Include "nlattr.h".
> > (decode_nla_*): New functions.
> > (decode_nlattr_with_data): Add decoders, size and opaque_data argument.
> > * netlink_sock_diag.c (decode_inet_diag_req_compat)
> > (decode_inet_diag_req_v2, decode_inet_diag_msg)
> > (decode_netlink_diag_msg, (decode_packet_diag_msg)
> > (decode_smc_diag_msg, decode_unix_diag_msg): Add decoders,
> > size and opaque_data arguments.
> > 
> > Co-authored-by: Fabien Siron <fabien.siron at epita.fr>
> > ---
> >  Makefile.am         |  1 +
> >  defs.h              |  3 ---
> >  netlink_sock_diag.c | 22 +++++++++++++------
> >  nlattr.c            | 63 +++++++++++++++++++++++++++++++++++++++++++++++++----
> >  nlattr.h            | 55 ++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 130 insertions(+), 14 deletions(-)
> >  create mode 100644 nlattr.h
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 132b5e1..a25e41d 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -174,6 +174,7 @@ strace_SOURCES =	\
> >  	netlink.c       \
> >  	netlink_sock_diag.c \
> >  	nlattr.c	\
> > +	nlattr.h	\
> >  	nsfs.c          \
> >  	nsfs.h          \
> >  	nsig.h		\
> > diff --git a/defs.h b/defs.h
> > index 19653f3..487e51b 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -632,9 +632,6 @@ tprint_iov_upto(struct tcb *, kernel_ulong_t len, kernel_ulong_t addr,
> >  
> >  extern void
> >  decode_netlink(struct tcb *, int fd, kernel_ulong_t addr, kernel_ulong_t len);
> > -extern void
> > -decode_nlattr(struct tcb *, kernel_ulong_t addr, kernel_ulong_t len,
> > -	      const struct xlat *, const char *);
> >  
> >  extern void tprint_open_modes(unsigned int);
> >  extern const char *sprint_open_modes(unsigned int);
> > diff --git a/netlink_sock_diag.c b/netlink_sock_diag.c
> > index f0e2f30..1443995 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>
> > @@ -146,7 +147,8 @@ decode_unix_diag_msg(struct tcb *const tcp,
> >  		tprints(", ");
> >  		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(msg)),
> >  			      len - NLA_ALIGN(sizeof(msg)),
> > -			      unix_diag_attrs, "UNIX_DIAG_???");
> > +			      unix_diag_attrs, "UNIX_DIAG_???",
> > +			      NULL, 0, NULL);
> >  	}
> >  }
> >  
> > @@ -233,7 +235,8 @@ decode_netlink_diag_msg(struct tcb *const tcp,
> >  		tprints(", ");
> >  		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(msg)),
> >  			      len - NLA_ALIGN(sizeof(msg)),
> > -			      netlink_diag_attrs, "NETLINK_DIAG_???");
> > +			      netlink_diag_attrs, "NETLINK_DIAG_???",
> > +			      NULL, 0, NULL);
> >  	}
> >  }
> >  
> > @@ -302,7 +305,8 @@ decode_packet_diag_msg(struct tcb *const tcp,
> >  		tprints(", ");
> >  		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(msg)),
> >  			      len - NLA_ALIGN(sizeof(msg)),
> > -			      packet_diag_attrs, "PACKET_DIAG_???");
> > +			      packet_diag_attrs, "PACKET_DIAG_???",
> > +			      NULL, 0, NULL);
> >  	}
> >  }
> >  
> > @@ -364,7 +368,8 @@ decode_inet_diag_req_compat(struct tcb *const tcp,
> >  		tprints(", ");
> >  		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(req)),
> >  			      len - NLA_ALIGN(sizeof(req)),
> > -			      inet_diag_req_attrs, "INET_DIAG_REQ_???");
> > +			      inet_diag_req_attrs, "INET_DIAG_REQ_???",
> > +			      NULL, 0, NULL);
> >  	}
> >  }
> >  
> > @@ -406,7 +411,8 @@ decode_inet_diag_req_v2(struct tcb *const tcp,
> >  		tprints(", ");
> >  		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(req)),
> >  			      len - NLA_ALIGN(sizeof(req)),
> > -			      inet_diag_req_attrs, "INET_DIAG_REQ_???");
> > +			      inet_diag_req_attrs, "INET_DIAG_REQ_???",
> > +			      NULL, 0, NULL);
> >  	}
> >  }
> >  
> > @@ -470,7 +476,8 @@ decode_inet_diag_msg(struct tcb *const tcp,
> >  		tprints(", ");
> >  		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(msg)),
> >  			      len - NLA_ALIGN(sizeof(msg)),
> > -			      inet_diag_attrs, "INET_DIAG_???");
> > +			      inet_diag_attrs, "INET_DIAG_???",
> > +			      NULL, 0, NULL);
> >  	}
> >  }
> >  
> > @@ -548,7 +555,8 @@ decode_smc_diag_msg(struct tcb *const tcp,
> >  		tprints(", ");
> >  		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(msg)),
> >  			      len - NLA_ALIGN(sizeof(msg)),
> > -			      smc_diag_attrs, "SMC_DIAG_???");
> > +			      smc_diag_attrs, "SMC_DIAG_???",
> > +			      NULL, 0, NULL);
> >  	}
> >  }
> >  #endif
> > diff --git a/nlattr.c b/nlattr.c
> > index 827f109..982e422 100644
> > --- a/nlattr.c
> > +++ b/nlattr.c
> > @@ -28,6 +28,7 @@
> >   */
> >  
> >  #include "defs.h"
> > +#include "nlattr.h"
> >  
> >  #include <linux/netlink.h>
> >  
> > @@ -66,7 +67,10 @@ decode_nlattr_with_data(struct tcb *tcp,
> >  			kernel_ulong_t addr,
> >  			kernel_ulong_t len,
> >  			const struct xlat *const table,
> > -			const char *const dflt)
> > +			const char *const dflt,
> > +			nla_decoder_t *decoders,
> > +			const unsigned int size,
> > +			void *const opaque_data)
> >  {
> >  	tprints("{");
> >  
> > @@ -76,7 +80,13 @@ decode_nlattr_with_data(struct tcb *tcp,
> >  
> >  	if (nla_len > NLA_HDRLEN) {
> >  		tprints(", ");
> > -		printstrn(tcp, addr + NLA_HDRLEN, nla_len - NLA_HDRLEN);
> > +		if (!decoders
> > +		    || nla->nla_type >= size
> > +		    || !decoders[nla->nla_type]
> > +		    || !decoders[nla->nla_type](tcp, addr + NLA_HDRLEN,
> > +						nla_len - NLA_HDRLEN,
> > +						opaque_data))
> > +			printstrn(tcp, addr + NLA_HDRLEN, len - NLA_HDRLEN);
> >  	}
> >  
> >  	tprints("}");
> > @@ -87,7 +97,10 @@ decode_nlattr(struct tcb *const tcp,
> >  	      kernel_ulong_t addr,
> >  	      kernel_ulong_t len,
> >  	      const struct xlat *const table,
> > -	      const char *const dflt)
> > +	      const char *const dflt,
> > +	      nla_decoder_t *decoders,
> > +	      const unsigned int size,
> > +	      void *const opaque_data)
> >  {
> >  	struct nlattr nla;
> >  	bool print_array = false;
> > @@ -115,7 +128,8 @@ decode_nlattr(struct tcb *const tcp,
> >  			print_array = true;
> >  		}
> >  
> > -		decode_nlattr_with_data(tcp, &nla, addr, len, table, dflt);
> > +		decode_nlattr_with_data(tcp, &nla, addr, len, table, dflt,
> > +					decoders, size, opaque_data);
> >  
> >  		if (!next_addr)
> >  			break;
> > @@ -129,3 +143,44 @@ decode_nlattr(struct tcb *const tcp,
> >  		tprints("]");
> >  	}
> >  }
> > +
> > +bool
> > +decode_nla_str(struct tcb *tcp, kernel_ulong_t addr,
> > +	       kernel_ulong_t len, void *const opaque_data)
> > +{
> > +	printstr(tcp, addr);
> > +
> > +	return true;
> > +}
> > +
> > +bool
> > +decode_nla_strn(struct tcb *tcp, kernel_ulong_t addr,
> > +		kernel_ulong_t len, void *const opaque_data)
> > +{
> > +	printstrn(tcp, addr, len);
> > +
> > +	return true;
> > +}
> > +
> > +#define DECODE_NLA_INTEGER(name, type, fmt)				\
> > +bool									\
> > +decode_nla_ ## name(struct tcb *tcp, kernel_ulong_t addr,		\
> > +		    kernel_ulong_t len, void *const opaque_data)	\
> > +{									\
> > +	type num;							\
> > +									\
> > +	if (len < sizeof(num))						\
> > +		printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);		\
> 
> Why not return false here and let the caller handle this?
> 
> > +	else if (!umove_or_printaddr(tcp, addr, &num))			\
> > +		tprintf(fmt, num);					\
> > +	return true;							\
> > +}
> > +
> > +DECODE_NLA_INTEGER(u8, uint8_t, "%" PRIu8)
> > +DECODE_NLA_INTEGER(u16, uint16_t, "%" PRIu16)
> > +DECODE_NLA_INTEGER(u32, uint32_t, "%" PRIu32)
> > +DECODE_NLA_INTEGER(u64, uint64_t, "%" PRIu64)
> > +DECODE_NLA_INTEGER(s8, int8_t, "%" PRId8)
> > +DECODE_NLA_INTEGER(s16, int16_t, "%" PRId16)
> > +DECODE_NLA_INTEGER(s32, int32_t, "%" PRId32)
> > +DECODE_NLA_INTEGER(s64, int64_t, "%" PRId64)
> > diff --git a/nlattr.h b/nlattr.h
> > new file mode 100644
> > index 0000000..edae56b
> > --- /dev/null
> > +++ b/nlattr.h
> > @@ -0,0 +1,55 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +typedef bool (*nla_decoder_t)(struct tcb *, kernel_ulong_t addr,
> > +			      kernel_ulong_t len, void *opaque_data);
> > +extern void
> > +decode_nlattr(struct tcb *,
> > +	      kernel_ulong_t addr,
> > +	      kernel_ulong_t len,
> > +	      const struct xlat *,
> > +	      const char *dflt,
> > +	      nla_decoder_t *,
> 
> This is a constant array, it has to be declared "const" like xlat array above.
> 
> > +	      const unsigned int size,
> 
> "const" in definitions of scalar types might be useful,
> but here in declarations it does not affect anything.
> 
> > +	      void *opaque_data);
> 
> Do you have any plans on using opaque_data, or is it added here
> just in case somebody might want to use it some day in the future?

I don't have any plans, decode rtnetlink attribute will pass family[1].

> 
> > +
> > +#define DECL_NLA(name)					\
> > +extern bool						\
> > +decode_nla_ ## name(struct tcb *, kernel_ulong_t addr,	\
> > +		    kernel_ulong_t len, void *)
> > +DECL_NLA(u8);
> > +DECL_NLA(u16);
> > +DECL_NLA(u32);
> > +DECL_NLA(u64);
> > +DECL_NLA(s8);
> > +DECL_NLA(s16);
> > +DECL_NLA(s32);
> > +DECL_NLA(s64);
> > +DECL_NLA(str);
> > +DECL_NLA(strn);

[1]https://github.com/Saruta/strace/commit/bd7ae5b28e300db6ec89712243fc29d1f4571bd8

--
JingPiao Chen




More information about the Strace-devel mailing list