On Mon, Jun 13, 2016 at 02:37:22PM +0000, Fabien Siron wrote:
> This commit introduces a general socket netlink parser which prints
> the header and a string for the remaining part of the buffer. It doesn't
> handle all the netlink flags and types because the parser needs more
> information. It will be done soon.
> 
> * net.c (get_family): New function.
> (do_msghdr): Call decode_iov_netlink.
> * netlink.c: New file.
> (decode_netlink): New function.
> (decode_iov_netlink): New function.
> * defs.h (decode_iov_netlink): Add.
> * Makefile.am (strace_SOURCES): Add netlink.c.
> ---
>  Makefile.am           |  1 +
>  defs.h                |  4 +++
>  net.c                 | 60 ++++++++++++++++++++++++++++-----
>  netlink.c             | 93 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  util.c                |  2 +-
>  xlat/netlink_flags.in |  6 ++++
>  xlat/netlink_types.in |  4 +++
>  7 files changed, 161 insertions(+), 9 deletions(-)
>  create mode 100644 netlink.c
>  create mode 100644 xlat/netlink_flags.in
>  create mode 100644 xlat/netlink_types.in
> 
> diff --git a/Makefile.am b/Makefile.am
> index 77e0cc8..ab0e200 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -156,6 +156,7 @@ strace_SOURCES =  \
>       mtd.c           \
>       native_defs.h   \
>       net.c           \
> +     netlink.c       \
>       numa.c          \
>       open.c          \
>       or1k_atomic.c   \
> diff --git a/defs.h b/defs.h
> index 594a292..134e0fe 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -542,6 +542,7 @@ extern const char *signame(const int);
>  extern void pathtrace_select(const char *);
>  extern int pathtrace_match(struct tcb *);
>  extern int getfdpath(struct tcb *, int, char *, unsigned);
> +extern int getfdproto(struct tcb *, int);
>  
>  extern const char *xlookup(const struct xlat *, const uint64_t);
>  extern const char *xlat_search(const struct xlat *, const size_t, const 
> uint64_t);
> @@ -651,6 +652,9 @@ extern const char *sprintsigmask_n(const char *, const 
> void *, unsigned int);
>  extern void printsignal(int);
>  extern void tprint_iov(struct tcb *, unsigned long, unsigned long, int 
> decode_iov);
>  extern void tprint_iov_upto(struct tcb *, unsigned long, unsigned long, int 
> decode_iov, unsigned long);
> +extern void decode_iov_netlink_or_printaddr(struct tcb *, unsigned long,
> +                                         unsigned long, unsigned long);
> +extern void decode_netlink_or_printaddr(struct tcb *, unsigned long, 
> unsigned long);

There are no decode_netlink* functions that don't do printaddr,
so the "_or_printaddr" suffix looks redundant here.

Also, I'd rather rename decode_iov_netlink to decode_netlink_iov.

>  extern void tprint_open_modes(unsigned int);
>  extern const char *sprint_open_modes(unsigned int);
>  extern void print_seccomp_filter(struct tcb *, unsigned long);
> diff --git a/net.c b/net.c
> index 8cc97da..09f375e 100644
> --- a/net.c
> +++ b/net.c
> @@ -297,6 +297,39 @@ printsock(struct tcb *tcp, long addr, int addrlen)
>       print_sockaddr(tcp, &addrbuf, addrlen);
>  }
>  
> +#include "socketutils.h"
> +
> +static void
> +printsockbuf(struct tcb *tcp, int fd, long addr, long addrlen)
> +{
> +     int proto = getfdproto(tcp, fd);

This operation costs a few syscalls so it shouldn't be attempted
unless show_fd_path > 1.

> +     switch (proto) {
> +     case NETLINK:
> +             decode_netlink_or_printaddr(tcp, addr, addrlen);
> +             break;
> +     default:
> +             printstr(tcp, addr, addrlen);
> +     }
> +}
> +
> +static int
> +get_family(struct tcb *tcp, long addr, int addrlen)
> +{
> +     sockaddr_buf_t addrbuf;
> +
> +     if (addrlen < 2)
> +             return -1;
> +     if (addrlen > (int) sizeof(addrbuf))
> +             addrlen = sizeof(addrbuf);
> +
> +     memset(&addrbuf, 0, sizeof(addrbuf));
> +     if (umoven(tcp, addr, addrlen, addrbuf.pad) == -1)
> +             return -1;
> +
> +     return addrbuf.sa.sa_family;
> +}

As this information has already been fetched by printsock,
why do it the second time in a row?

> +
>  #include "xlat/scmvals.h"
>  #include "xlat/ip_cmsg_types.h"
>  
> @@ -598,8 +631,16 @@ do_msghdr(struct tcb *tcp, struct msghdr *msg, unsigned 
> long data_size)
>       printsock(tcp, (long)msg->msg_name, msg->msg_namelen);
>  
>       tprintf(", msg_iov(%lu)=", (unsigned long)msg->msg_iovlen);
> -     tprint_iov_upto(tcp, (unsigned long)msg->msg_iovlen,
> -                (unsigned long)msg->msg_iov, 1, data_size);
> +#ifdef AF_NETLINK
> +     if (get_family(tcp, (long) msg->msg_name, msg->msg_namelen)
> +         == AF_NETLINK)
> +             decode_iov_netlink_or_printaddr(tcp,
> +                             (unsigned long) msg->msg_iovlen,
> +                             (unsigned long) msg->msg_iov, data_size);
> +     else
> +#endif
> +             tprint_iov_upto(tcp, (unsigned long)msg->msg_iovlen,
> +                             (unsigned long)msg->msg_iov, 1, data_size);
>  
>  #ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
>       tprintf(", msg_controllen=%lu", (unsigned long)msg->msg_controllen);
> @@ -886,7 +927,7 @@ SYS_FUNC(send)
>  {
>       printfd(tcp, tcp->u_arg[0]);
>       tprints(", ");
> -     printstr(tcp, tcp->u_arg[1], tcp->u_arg[2]);
> +     printsockbuf(tcp, tcp->u_arg[0], tcp->u_arg[1], tcp->u_arg[2]);
>       tprintf(", %lu, ", tcp->u_arg[2]);
>       /* flags */
>       printflags(msg_flags, tcp->u_arg[3], "MSG_???");
> @@ -898,7 +939,7 @@ SYS_FUNC(sendto)
>  {
>       printfd(tcp, tcp->u_arg[0]);
>       tprints(", ");
> -     printstr(tcp, tcp->u_arg[1], tcp->u_arg[2]);
> +     printsockbuf(tcp, tcp->u_arg[0], tcp->u_arg[1], tcp->u_arg[2]);
>       tprintf(", %lu, ", tcp->u_arg[2]);
>       /* flags */
>       printflags(msg_flags, tcp->u_arg[3], "MSG_???");
> @@ -947,10 +988,11 @@ SYS_FUNC(recv)
>               printfd(tcp, tcp->u_arg[0]);
>               tprints(", ");
>       } else {
> -             if (syserror(tcp))
> +             if (syserror(tcp)) {
>                       printaddr(tcp->u_arg[1]);
> -             else
> -                     printstr(tcp, tcp->u_arg[1], tcp->u_rval);
> +             } else
> +                     printsockbuf(tcp, tcp->u_arg[0], tcp->u_arg[1],
> +                             tcp->u_rval);
>  
>               tprintf(", %lu, ", tcp->u_arg[2]);
>               printflags(msg_flags, tcp->u_arg[3], "MSG_???");
> @@ -966,11 +1008,13 @@ SYS_FUNC(recvfrom)
>               printfd(tcp, tcp->u_arg[0]);
>               tprints(", ");
>       } else {
> +
>               /* buf */
>               if (syserror(tcp)) {
>                       printaddr(tcp->u_arg[1]);
>               } else {
> -                     printstr(tcp, tcp->u_arg[1], tcp->u_rval);
> +                     printsockbuf(tcp, tcp->u_arg[0], tcp->u_arg[1],
> +                             tcp->u_rval);
>               }
>               /* len */
>               tprintf(", %lu, ", tcp->u_arg[2]);
> diff --git a/netlink.c b/netlink.c
> new file mode 100644
> index 0000000..738c639
> --- /dev/null
> +++ b/netlink.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright (c) 2016 Fabien Siron <fabien.si...@epita.fr>
> + * 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 <sys/socket.h>
> +#if defined(HAVE_LINUX_NETLINK_H)
> +# include <linux/netlink.h>
> +#endif

socketutils.c already includes <linux/netlink.h> unconditionally.

> +#include "xlat/netlink_flags.h"
> +#include "xlat/netlink_types.h"
> +
> +void
> +decode_netlink_or_printaddr(struct tcb *tcp, unsigned long addr,
> +                         unsigned long size)
> +{
> +#ifdef AF_NETLINK

socketutils.c already uses AF_NETLINK unconditionally.

> +     struct nlmsghdr nlmsghdr;
> +     memset(&nlmsghdr, 0, sizeof(nlmsghdr));

You can assume that compiler supports C99.

> +     if (umoven(tcp, addr, sizeof(nlmsghdr), &nlmsghdr) == -1)
> +             return;

Can "size" be lower than sizeof(nlmsghdr)?

Note that this function has name decode_netlink_or_printaddr but it
doesn't do any printaddr.
Looks like umove_or_printaddr is more appropriate here.

> +     tprintf("{{len=%u, type=", nlmsghdr.nlmsg_len);
> +     if (nlmsghdr.nlmsg_type < NLMSG_MIN_TYPE)
> +             printxval(netlink_types, nlmsghdr.nlmsg_type, "NLMSG_???");
> +     else
> +             tprintf("%u", nlmsghdr.nlmsg_type);

printxval can be invoked here unconditionally.

> +     tprints(", flags=");
> +     printflags(netlink_flags, nlmsghdr.nlmsg_flags, "NLM_F_???");
> +     /* manage get/new requests */
> +
> +     tprintf(", seq=%u, pid=%u}, ", nlmsghdr.nlmsg_seq,
> +             nlmsghdr.nlmsg_pid);
> +
> +     if (size - sizeof(struct nlmsghdr) > 0)
> +             printstr(tcp, addr + sizeof(struct nlmsghdr),
> +                      size - sizeof(struct nlmsghdr));

An extra comma will be printed when this condition is false.

> +     tprints("}");
> +
> +#else
> +     printaddr(addr);
> +#endif
> +}
> +
> +void
> +decode_iov_netlink_or_printaddr(struct tcb *tcp, unsigned long len,
> +                             unsigned long addr, unsigned long data_size)
> +{
> +     unsigned long iov[2];
> +     unsigned int i;
> +
> +     if (!len) {
> +             tprints("[]");
> +             return;
> +     }
> +
> +     for (i = 0; i < len; ++i) {
> +             if(umove_or_printaddr(tcp, addr + i * sizeof(iov), iov))
> +                     return;
> +
> +             if (iov[1] < sizeof(struct nlmsghdr)) {
> +                     tprintf("len(%lu)", len);
> +                     printaddr(addr);
> +                     return;
> +             }
> +             decode_netlink_or_printaddr(tcp, iov[0], iov[1]);
> +     }
> +}

Please do not reinvent print_array.  See e.g. tprint_iov_upto.


-- 
ldv

Attachment: pgpP7UK51eJuk.pgp
Description: PGP signature

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to