netdev - Re: [PATCH net-next v5] rtnetlink: Support fine-grained netdevice bulk deletion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <84166fe9-37f1-2c99-2caf-c68dcc5a370c@6wind.com>
Date:   Mon, 6 Dec 2021 09:25:17 +0100
From:   Nicolas Dichtel <nicolas.dichtel@...nd.com>
To:     Lahav Schlesinger <lschlesinger@...venets.com>,
        netdev@...r.kernel.org
Cc:     kuba@...nel.org, dsahern@...il.com, nikolay@...dia.com
Subject: Re: [PATCH net-next v5] rtnetlink: Support fine-grained netdevice
 bulk deletion

Le 05/12/2021 à 10:36, Lahav Schlesinger a écrit :
Some comments below, but please, keep the people who replied to previous
versions of this patch in cc.

[snip]

> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index eebd3894fe89..68fcde9c0c5e 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -348,6 +348,7 @@ enum {
>  	IFLA_PARENT_DEV_NAME,
>  	IFLA_PARENT_DEV_BUS_NAME,
>  
> +	IFLA_IFINDEX,
nit: maybe the previous blank line sit better after this new attribute (and
before __IFLA_MAX)?

>  	__IFLA_MAX
>  };
>  
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index fd030e02f16d..5165cc699d97 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -37,6 +37,7 @@
>  #include <linux/pci.h>
>  #include <linux/etherdevice.h>
>  #include <linux/bpf.h>
> +#include <linux/sort.h>
>  
>  #include <linux/uaccess.h>
>  
> @@ -1880,6 +1881,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>  	[IFLA_PROTO_DOWN_REASON] = { .type = NLA_NESTED },
>  	[IFLA_NEW_IFINDEX]	= NLA_POLICY_MIN(NLA_S32, 1),
>  	[IFLA_PARENT_DEV_NAME]	= { .type = NLA_NUL_STRING },
> +	[IFLA_IFINDEX]		= { .type = NLA_S32 },
Same policy than IFLA_NEW_IFINDEX to refuse negative ifindex.

>  };
>  
>  static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
> @@ -3050,6 +3052,78 @@ static int rtnl_group_dellink(const struct net *net, int group)
>  	return 0;
>  }
>  
> +static int dev_ifindex_cmp(const void *a, const void *b)
> +{
> +	struct net_device * const *dev1 = a, * const *dev2 = b;
> +
> +	return (*dev1)->ifindex - (*dev2)->ifindex;
> +}
> +
> +static int rtnl_ifindex_dellink(struct net *net, struct nlattr *head, int len,
> +				struct netlink_ext_ack *extack)
> +{
> +	int i = 0, num_devices = 0, rem;
> +	struct net_device **dev_list;
> +	const struct nlattr *nla;
> +	LIST_HEAD(list_kill);
> +	int ret;
> +
> +	nla_for_each_attr(nla, head, len, rem) {
> +		if (nla_type(nla) == IFLA_IFINDEX)
> +			num_devices++;
> +	}
> +
> +	dev_list = kmalloc_array(num_devices, sizeof(*dev_list), GFP_KERNEL);
> +	if (!dev_list)
> +		return -ENOMEM;
> +
> +	nla_for_each_attr(nla, head, len, rem) {
> +		const struct rtnl_link_ops *ops;
> +		struct net_device *dev;
> +		int ifindex;
> +
> +		if (nla_type(nla) != IFLA_IFINDEX)
> +			continue;
> +
> +		ifindex = nla_get_s32(nla);
> +		ret = -ENODEV;
> +		dev = __dev_get_by_index(net, ifindex);
> +		if (!dev) {
> +			NL_SET_ERR_MSG_ATTR(extack, nla, "Unknown ifindex");
It would be nice to have the ifindex in the error message. This message does not
give more information than "ENODEV".

> +			goto out_free;
> +		}
> +
> +		ret = -EOPNOTSUPP;
> +		ops = dev->rtnl_link_ops;
> +		if (!ops || !ops->dellink) {
> +			NL_SET_ERR_MSG_ATTR(extack, nla, "Device cannot be deleted");
Same here.


Thank you,
Nicolas