Re: [PATCH net-next v2 3/3] netlink: add validation of NLA_F_NESTED flag

On Thu, 2019-05-02 at 16:15 +0200, Michal Kubecek wrote:

Add new validation flag NL_VALIDATE_NESTED which adds three
consistency
checks of NLA_F_NESTED_FLAG:

  - the flag is set on attributes with NLA_NESTED{,_ARRAY} policy
  - the flag is not set on attributes with other policies except
NLA_UNSPEC
  - the flag is set on attribute passed to nla_parse_nested()

Signed-off-by: Michal Kubecek <redacted>

v2: change error messages to mention NLA_F_NESTED explicitly
---
 include/net/netlink.h | 11 ++++++++++-
 lib/nlattr.c          | 15 +++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

Hi,


libnl3 does currently not ever set NLA_F_NESTED flag.

That means, nla_nest_start() will not work as it used to.
https://github.com/thom311/libnl/blob/65b3dd5ac2d5de4c7a0c64e430596d9d27973527/lib/attr.c#L902

As workaround, one could call

  nla_nest_start(msg, NLA_F_NESTED | attr);


Of course, that is a bug in libnl3 that should be fixed. But it seems
quite unfortunate to me.


Does this flag and strict validation really provide any value? Commonly a netlink message
is a plain TLV blob, and the meaning depends entirely on the policy.

What I mean is that for example

  NLA_PUT_U32 (msg, ATTR_IFINDEX, (uint32_t) ifindex)
  NLA_PUT_STRING (msg, ATTR_IFNAME, "net")

results in a 4 bytes payload that does not encode whether the data is a number or
a string.

Why is it valuable in this case to encode additional type information inside the message,
when it's commonly not done and also not necessary?


best,
Thomas