Re: netlink: 16 bytes leftover after parsing attributes in process `ip'.

[Jiri Benc <jbenc@redhat.com>]

On Tue, 25 Sep 2018 11:49:10 +0200, Christian Brauner wrote:
> So if people really want to hide this issue as much as we can then we
> can play the guessing game. I could send a patch that roughly does the
> following:
> 
> if (nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg))
>         guessed_header_len = sizeof(struct ifaddrmsg);
> else
>         guessed_header_len = sizeof(struct ifinfomsg);
> 
> This will work since sizeof(ifaddrmsg) == 8 and sizeof(ifinfomsg) == 16.
> The only valid property for RTM_GETADDR requests is IFA_TARGET_NETNSID.
> This propert is a __s32 which should bring the message up to 12 bytes
> (not sure about alignment requiremnts and where we might wend up ten)
> which is still less than the 16 bytes without that property from
> ifinfomsg. That's a hacky hacky hack-hack and will likely work but will
> break when ifaddrmsg grows a new member or we introduce another property
> that is valid in RTM_GETADDR requests. It also will not work cleanly
> when users stuff additional properties in there that are vaif (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,lid for the
> address family but are not used int RTM_GETADDR requests.

I'd expect that any potential existing code that makes use of other
attributes already assumes ifaddrmsg. Hence, if the nlmsg_len is >
sizeof(ifinfomsg), you can be sure that there are attributes and thus
the struct used was ifaddrmsg.

So, in order for RTM_GETADDR to work reliably with attributes, you have
to ensure that the length is > sizeof(ifinfomsg).

This can be achieved by putting IFA_TARGET_NETNSID into a nested
attribute. Just define IFA_EXTENDED (feel free to invent a better name,
of course) and put IFA_TARGET_NETNSID inside. Then in the code, attempt
to parse only when the size is large enough:

	if (nlmsg_len(cb->nlh) > sizeof(struct ifinfomsg)) {
		int err;
 
		err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
		                  IFA_MAX, ifa_ipv6_policy, NULL);
		if (err < 0)
			return err;
		if (tb[IFA_EXTENDED]) {
			...parse the nested attribute...
			if (tb_nested[IFA_TARGET_NETNSID]) {
				...etc...
			}
		}
	}

Another option is forcing the user space to add another attribute, for
example, IFA_FLAGS_PRESENT, and attempt parsing only when it is
present. The logic would then be:

	if (nlmsg_len(cb->nlh) > sizeof(struct ifinfomsg)) {
		int err;
 
		err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
		                  IFA_MAX, ifa_ipv6_policy, NULL);
		if (err < 0)
			return err;
		if (tb[IFA_FLAGS_PRESENT] && tb[IFA_TARGET_NETNSID]) {
			...etc...
		}
	}

 Jiri