Re: [PATCH v4] netlink: decode libudev netlink header

[Dmitry V. L. <ld...@al...>]

On Tue, Jan 09, 2018 at 01:49:36AM +0530, Harsha Sharma wrote:
> * defs.h (decode_netlink_kobject_uevent): New prototype.
> * netlink.c (decode_netlink): Decode family kobject_uevent.
> * netlink_kobject_uevent.h: New file.
> * netlink_object_uevent.c: New file.
> * Makefile.am (strace_SOURCES): Add them.
> 
> Signed-off-by: Harsha Sharma <har...@gm...>
> ---
> Changes in v4:
> * Minor changes in decode_netlink_kobject_uevent
> 
> Changes in v3:
> * Change changelog
> * minor changes in decode_netlink_kobject_uevent
> * omit irrelevant comments in netlink_kobject_uevent.h
> 
> Changes in v2:
> * New file netlink_kobject_uevent.c
> * change location of netlink_kobject_uevent.h
> * add comments in netlink_kobject_uevent.h
> * add new files in Makefile.am
> * add func decode_netlink_kobject_uevent in defs.h
> * Use macros from print_fields.h instead of tprintf
> * minor changes in decode_netlink_kobject_uevent (including name)
> 
>  Makefile.am              |  2 ++
>  defs.h                   |  3 +++
>  netlink.c                |  2 +-
>  netlink_kobject_uevent.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
>  netlink_kobject_uevent.h | 17 +++++++++++++
>  5 files changed, 87 insertions(+), 1 deletion(-)
>  create mode 100644 netlink_kobject_uevent.c
>  create mode 100644 netlink_kobject_uevent.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index 670046a5..719516e1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -194,6 +194,8 @@ strace_SOURCES =	\
>  	netlink.c       \
>  	netlink.h	\
>  	netlink_crypto.c \
> +	netlink_kobject_uevent.c \
> +	netlink_kobject_uevent.h \
>  	netlink_sock_diag.h \
>  	netlink_inet_diag.c \
>  	netlink_netlink_diag.c \
> diff --git a/defs.h b/defs.h
> index e7d7b2df..6aafeb28 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -694,6 +694,9 @@ DECL_NETLINK(route);
>  DECL_NETLINK(selinux);
>  DECL_NETLINK(sock_diag);
>  
> +extern void
> +decode_netlink_kobject_uevent(struct tcb *, kernel_ulong_t addr,
> +			      kernel_ulong_t len);
>  extern int tv_nz(const struct timeval *);
>  extern int tv_cmp(const struct timeval *, const struct timeval *);
>  extern double tv_float(const struct timeval *);
> diff --git a/netlink.c b/netlink.c
> index beb6ea4a..a714d29a 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -629,7 +629,7 @@ decode_netlink(struct tcb *const tcp,
>  	const int family = get_fd_nl_family(tcp, fd);
>  
>  	if (family == NETLINK_KOBJECT_UEVENT) {
> -		printstrn(tcp, addr, len);
> +		decode_netlink_kobject_uevent(tcp, addr, len);
>  		return;
>  	}
>  
> diff --git a/netlink_kobject_uevent.c b/netlink_kobject_uevent.c
> new file mode 100644
> index 00000000..813638da
> --- /dev/null
> +++ b/netlink_kobject_uevent.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright (c) 2018 Harsha Sharma <har...@gm...>
> + * Copyright (c) 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.
> + */
> +
> +#include "defs.h"
> +#include "print_fields.h"
> +#include "netlink_kobject_uevent.h"
> +
> +#include <arpa/inet.h>
> +
> +void
> +decode_netlink_kobject_uevent(struct tcb *tcp, kernel_ulong_t addr,
> +			      kernel_ulong_t len)
> +{
> +	struct udev_monitor_netlink_header uh;
> +	const char *prefix = "libudev";
> +
> +	if (len >= sizeof(uh)) {
> +		if (!umove_or_printaddr(tcp, addr, &uh)) {
> +			if (strcmp(uh.prefix, prefix) == 0) {
> +				PRINT_FIELD_CSTRING("{", uh, prefix);
> +				tprintf(", magic=htonl(%#x)", ntohl(uh.magic));
> +				PRINT_FIELD_U(", ", uh, header_size);
> +				PRINT_FIELD_U(", ", uh, properties_off);
> +				PRINT_FIELD_U(", ", uh, properties_len);
> +				PRINT_FIELD_X(", ", uh, filter_subsystem_hash);
> +				PRINT_FIELD_X(", ", uh, filter_devtype_hash);
> +				PRINT_FIELD_X(", ", uh, filter_tag_bloom_hi);
> +				PRINT_FIELD_X(", ", uh, filter_tag_bloom_lo);

Sorry but you must have missed my comment:
all these filter_* fields are hashes stored in network byte order.

> +				tprints("}");
> +				printstrn(tcp, addr - sizeof(uh),
> +					  len - sizeof(uh));

This would print {...}"..." which is not a correct output.

I think it's time to add a test for this new parser.


-- 
ldv