[PATCH v2 01/16] netlink: introduce a dummy netlink attributes parser

[Dmitry V. Levin]

On Sun, Jun 18, 2017 at 05:54:05PM +0800, JingPiao Chen wrote:
> On Sun, Jun 18, 2017 at 11:19:50AM +0300, Dmitry V. Levin wrote:
> > On Sun, Jun 18, 2017 at 01:18:28PM +0800, JingPiao Chen wrote:
> > > * defs.h (decode_nlattr): New prototype.
> > 
> > This is moved to nlattr.h by patch 13/16; why not put it there in the
> > first place?
> > 
> > > * linux/unix_diag.h (UNIX_DIAG_*): New enum.
> > > * nlattr.c: New file.
> > > * Makefile.am (strace_SOURCES): Add it.
> > > * netlink_sock_diag.c: Include "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>
> > > ---
> > >  Makefile.am             |   1 +
> > >  defs.h                  |   3 ++
> > >  linux/unix_diag.h       |  11 +++-
> > >  netlink_sock_diag.c     |   8 +++
> > >  nlattr.c                | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  xlat/unix_diag_attrs.in |   8 +++
> > >  6 files changed, 160 insertions(+), 2 deletions(-)
> > >  create mode 100644 nlattr.c
> > >  create mode 100644 xlat/unix_diag_attrs.in
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index e22d480..132b5e1 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -173,6 +173,7 @@ strace_SOURCES =	\
> > >  	net.c		\
> > >  	netlink.c       \
> > >  	netlink_sock_diag.c \
> > > +	nlattr.c	\
> > >  	nsfs.c          \
> > >  	nsfs.h          \
> > >  	nsig.h		\
> > > diff --git a/defs.h b/defs.h
> > > index 487e51b..19653f3 100644
> > > --- a/defs.h
> > > +++ b/defs.h
> > > @@ -632,6 +632,9 @@ 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/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..e515810 100644
> > > --- a/netlink_sock_diag.c
> > > +++ b/netlink_sock_diag.c
> > > @@ -55,6 +55,7 @@
> > >  # include "xlat/smc_states.h"
> > >  #endif
> > >  
> > > +#include "xlat/unix_diag_attrs.h"
> > >  #include "xlat/unix_diag_show.h"
> > >  
> > >  static void
> > > @@ -135,6 +136,13 @@ decode_unix_diag_msg(struct tcb *const tcp,
> > >  	} else
> > >  		tprints("...");
> > >  	tprints("}");
> > > +
> > > +	if (len > NLA_ALIGN(sizeof(msg))) {
> > > +		tprints(", ");
> > > +		decode_nlattr(tcp, addr + NLA_ALIGN(sizeof(msg)),
> > > +			      len - NLA_ALIGN(sizeof(msg)),
> > > +			      unix_diag_attrs, "UNIX_DIAG_???");
> > > +	}
> > 
> > Please move this to "!umoven_or_printaddr" branch of the "if" statement
> > above.
> Your expected output is
> 
> {udiag_family=AF_UNIX, udiag_type=SOCK_STREAM
> , udiag_state=TCP_FIN_WAIT1, udiag_ino=0
> , udiag_cookie=[0, 0], {nla=0, nla_type=INET_DIAG_NONE}}
> 
> or
> 
> {udiag_family=AF_UNIX, udiag_type=SOCK_STREAM
> , udiag_state=TCP_FIN_WAIT1, udiag_ino=0
> , udiag_cookie=[0, 0]}, {nla=0, nla_type=INET_DIAG_NONE}
> 
> second output I using following implementation:
> 
> 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
> @@ -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_???");
> +	}

Yes, this implementation is correct: we shouldn't try decoding nlattrs
if the basic decoding was not successful.


-- 
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/20170619/11a13ad1/attachment.bin>