Re: [B.A.T.M.A.N.] [PATCHv2] batctl: Use netlink to replace some of debugfs

Sven Eckelmann
2016-05-03T07:07:35Z

[-- Attachment #1: Type: text/plain, Size: 2310 bytes --]

[...]
> +ifeq ($(origin LIBNL_GENL_CFLAGS) $(origin LIBNL_GENL_LDLIBS), undefined undefined)
> + LIBNL_GENL_NAME ?= libnl-genl-3.0
> + ifeq ($(shell $(PKG_CONFIG) --modversion $(LIBNL_GENL_NAME) 2>/dev/null),)
> + $(error No $(LIBNL_GENL_NAME) development libraries found!)
> + endif
> + LIBNL_GENL_CFLAGS += $(shell $(PKG_CONFIG) --cflags $(LIBNL_GENL_NAME))
> + LIBNL_GENL_LDLIBS += $(shell $(PKG_CONFIG) --libs $(LIBNL_GENL_NAME))
> +endif
> +CFLAGS += $(LIBNL_GENL_CFLAGS)
> +LDLIBS += $(LIBNL_GENL_LDLIBS)

@Simon, @Matthias, @Antonio, @Marek: Should the header file be included in
batctl like nl80211.h in iw. Or should we always require the current kernel
header files installed to build batctl?

[...]
> +struct mandatory_attr {
> + int attr;
> + int datalen;
> +};
> +
> +static int last_err;
> +
> +static int invalidate_mandatory_attrs(struct nlattr *attrs[],
> + const struct mandatory_attr *mandatory[],
> + int num)
> +{
> + int i;
> + int len;
> +
> + for (i = 0; i < num; i++) {
> + if (!attrs[mandatory[i]->attr])
> + return EINVAL;
> + len = nla_len(attrs[mandatory[i]->attr]);
> + if (mandatory[i]->datalen && (len != mandatory[i]->datalen))
> + return EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct mandatory_attr mandatory_attr_version= {
> + BATADV_ATTR_VERSION, 0 };
[...]
> +static const struct mandatory_attr *info_hard_mandatory[] = {
> + &mandatory_attr_version,
> + &mandatory_attr_algo_name,
> + &mandatory_attr_hard_ifname,
> + &mandatory_attr_hard_address,
> +};
> +
> +static int info_callback(struct nl_msg *msg, void *arg __unused)
> +{
[...]
> + if (nla_parse(attrs, BATADV_ATTR_MAX, genlmsg_attrdata(ghdr, 0),
> + genlmsg_len(ghdr), NULL)) {
> + fputs("Received invalid data from kernel.", stderr);
> + exit(1);
> + }
> +
> + if (invalidate_mandatory_attrs(attrs, info_mandatory,
> + ARRAY_SIZE(info_mandatory))) {
> + fputs("Missing/invalid attributes from kernel\n", stderr);
> + exit(1);
> + }

Interesting idea to check the mandatory attributes with a common function. But
shouldn't be the length checked by the nl_parse(..., policy) [1]? This is
especially important because you don't check the type.

Kind regards,
Sven

[1] http://www.infradead.org/~tgr/libnl/doc/core.html#core_attr_validation

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]