[PATCH v8 3/4] Netlink: handle multi netlink messages

[Dmitry V. Levin]

On Wed, Jun 29, 2016 at 12:20:00PM +0000, Fabien Siron wrote:
> +static unsigned long
> +next_nlmsg(struct nlmsghdr *nlmsghdr, unsigned long addr, unsigned long *len) {
> +	if (NLMSG_ALIGN(nlmsghdr->nlmsg_len) == 0 ||
> +	    NLMSG_ALIGN(nlmsghdr->nlmsg_len) > *len) {
> +		*len = NLMSG_ALIGN(nlmsghdr->nlmsg_len);
> +		return 0;
> +	}

*len may increase as result

> +	*len -= NLMSG_ALIGN(nlmsghdr->nlmsg_len);
> +
> +	if (addr + NLMSG_ALIGN(nlmsghdr->nlmsg_len) > addr) {
> +		return addr + NLMSG_ALIGN(nlmsghdr->nlmsg_len);
> +	} else {
> +		*len = 0;
> +		return 0;
> +	}
> +}

Could it be written in a more clear and robust way?  For example,

unsigned long nlmsg_len = NLMSG_ALIGN(nlmsghdr->nlmsg_len);

if (nlmsg_len < sizeof(struct nlmsghdr)) {
	addr = 0;
	nlmsg_len = sizeof(struct nlmsghdr);
}

if (*len >= nlmsg_len)
	*len -= nlmsg_len;
else
	*len = 0;

if (addr && addr + nlmsg_len > addr)
	return addr + nlmsg_len;
else
	return 0;

> +static void
> +decode_netlink_msg(struct tcb *tcp, struct nlmsghdr *nlmsghdr,
> +		   unsigned long addr, unsigned long size)
> +{
>  	if (size < sizeof(struct nlmsghdr)) {
>  		printstr(tcp, addr, size);
>  		return;
>  	}
> -	if (umove_or_printaddr(tcp, addr, &nlmsghdr))
> -		return;
>  
> -	tprintf("{{len=%u, type=", nlmsghdr.nlmsg_len);
> +	tprintf("{{len=%u, type=", nlmsghdr->nlmsg_len);
>  
> -	printxval(netlink_types, nlmsghdr.nlmsg_type, "NLMSG_???");
> +	printxval(netlink_types, nlmsghdr->nlmsg_type, "NLMSG_???");
>  
>  	tprints(", flags=");
> -	printflags(netlink_flags, nlmsghdr.nlmsg_flags, "NLM_F_???");
> +	printflags(netlink_flags, nlmsghdr->nlmsg_flags, "NLM_F_???");
>  	/* manage get/new requests */
>  
> -	tprintf(", seq=%u, pid=%u}", nlmsghdr.nlmsg_seq,
> -		nlmsghdr.nlmsg_pid);
> +	tprintf(", seq=%u, pid=%u}", nlmsghdr->nlmsg_seq,
> +		nlmsghdr->nlmsg_pid);
>  
> -	if (size - sizeof(struct nlmsghdr) > 0) {
> +	if (nlmsghdr->nlmsg_len - sizeof(struct nlmsghdr) > 0) {
>  		tprints(", ");
> +
>  		printstr(tcp, addr + sizeof(struct nlmsghdr),
> -			 size - sizeof(struct nlmsghdr));
> +			 nlmsghdr->nlmsg_len - sizeof(struct nlmsghdr));
>  	}
>  
>  	tprints("}");
>  }
> +
> +void
> +decode_netlink(struct tcb *tcp, unsigned long addr, unsigned long total_size) {
> +	struct nlmsghdr nlmsghdr;
> +	unsigned long elt, size = total_size;
> +	int print_array = 0;
> +
> +	for (elt = 0; fetch_nlmsg(tcp, &nlmsghdr, addr, size);
> +	     addr = next_nlmsg(&nlmsghdr, addr, &size), elt++) {
> +		if (elt == max_strlen && abbrev(tcp)) {
> +			tprints("...");
> +			break;
> +		}
> +		if (nlmsghdr.nlmsg_len < sizeof(struct nlmsghdr))
> +			break;

This edition of decode_netlink will print nothing if addr == NULL,
or if the first nlmsghdr.nlmsg_len < sizeof(struct nlmsghdr).

Please add to the test all pathological cases we discussed so far.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160705/b6c7885a/attachment.bin>