[PATCH v6 4/4] Netlink: handle multi netlink messages

[Dmitry V. Levin]

On Tue, Jun 21, 2016 at 02:42:46PM +0000, Fabien Siron wrote:
[...]
> +static unsigned long
> +nlmsg_next(struct nlmsghdr *nlmsghdr, unsigned long addr, unsigned long *len) {
> +	*len -= NLMSG_ALIGN(nlmsghdr->nlmsg_len);
> +
> +	if (NLMSG_ALIGN(nlmsghdr->nlmsg_len) == 0 ||
> +	    NLMSG_ALIGN(nlmsghdr->nlmsg_len) > *len)

The check is too late.

> +		return 0;
> +
> +	return (unsigned long)((addr) + NLMSG_ALIGN(nlmsghdr->nlmsg_len));
> +}

Why (addr)?  Is this cast to (unsigned long) really needed?
What's going to happen in case of integer overflow?

> +static void
> +decode_netlink_msg(struct tcb *tcp, unsigned long addr,
> +			    unsigned long size)
>  {
>  	struct nlmsghdr nlmsghdr;
>  
> @@ -57,8 +86,32 @@ decode_netlink(struct tcb *tcp, unsigned long addr, unsigned long size)
>  	if (size - sizeof(struct nlmsghdr) > 0) {
>  		tprints(", ");
>  		printstr(tcp, addr + sizeof(struct nlmsghdr),
> -			 size - sizeof(struct nlmsghdr));
> +			 nlmsghdr.nlmsg_len - sizeof(struct nlmsghdr));
>  	}
>  	tprints("}");
>  }

What's going to be printed if size > sizeof(struct nlmsghdr)
AND nlmsghdr.nlmsg_len == sizeof(struct nlmsghdr)?

> +
> +void
> +decode_netlink(struct tcb *tcp, unsigned long addr, unsigned long total_size) {
> +	struct nlmsghdr nlmsghdr;
> +	unsigned long elt, size = total_size;
> +
> +	for (elt = 0; nlmsg_fetch(tcp, &nlmsghdr, addr, size);
> +	     addr = nlmsg_next(&nlmsghdr, addr, &size), elt++) {
> +		if (elt == max_strlen && abbrev(tcp)) {
> +			tprints("...");
> +			break;
> +		}
> +		if (size != total_size)
> +			tprints(", ");
> +		else if (NLMSG_ALIGN(nlmsghdr.nlmsg_len) < total_size)
> +			tprints("[");
> +		decode_netlink_msg(tcp, addr, size);
> +	}
> +	if (nlmsghdr.nlmsg_len != (unsigned) -1 &&
> +	    nlmsghdr.nlmsg_len != 0 &&
> +	    NLMSG_ALIGN(nlmsghdr.nlmsg_len) < total_size) {
> +		tprints("]");
> +	}
> +}

This check doesn't look obvious; is it correct?
Is there a more clear way to implement this?


-- 
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/20160621/17e78298/attachment.bin>