netdev - Re: [RFC PATCH V2] ethtool: Add command to configure IOV features

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 for Android: free password hash cracker in your pocket

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

Date:	Sat, 08 Oct 2011 01:41:25 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Greg Rose <gregory.v.rose@...el.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [RFC PATCH V2] ethtool:  Add command to configure IOV features

On Thu, 2011-09-22 at 14:35 -0700, Greg Rose wrote:
> New command to allow configuration of IOV features such as the number of
> Virtual Functions to allocate for a given Physical Function interface,
> the number of semi-independent net devices to allocate from partitioned
> I/O resources in the PF and to set the number of queues per VF.
[...]
> @@ -266,6 +270,8 @@ static struct option {
>      { "-W", "--set-dump", MODE_SET_DUMP,
>  		"Set dump flag of the device",
>  		"		N\n"},
> +    { "-v", "--get-iov", MODE_GET_IOV, "Get IOV parameters", "\n" },
> +    { "-V", "--set_iov", MODE_SET_IOV, "Set IOV parameters", "[ N ]\n" },

Doesn't match the implementation.

[...]
> +static int do_get_iov(int fd, struct ifreq *ifr)
> +{
> +	int err;
> +	struct ethtool_iov_get_cmd iov_cmd;
> +
> +	iov_cmd.cmd = ETHTOOL_IOV_GET_CMD;
> +	ifr->ifr_data = (caddr_t)&iov_cmd;
> +	err = send_ioctl(fd, ifr);
> +	if (err < 0) {
> +		perror("Can not get current IOV mode\n");
> +		return 1;
> +	}
> +
> +	memcpy(&iov_cmd, ifr->ifr_data, sizeof(iov_cmd));

But ifr->ifr_data == &iov_cmd.  So this is both pointless and dangerous
(as memcpy() doesn't handle overlapping source and destination).

[...]
> +static int do_set_iov(int fd, struct ifreq *ifr)
> +{
> +	int err;
> +	struct ethtool_iov_set_cmd iov_cmd;
> +
> +	if (iov_changed) {
> +		iov_cmd.cmd = ETHTOOL_IOV_SET_CMD;
> +		if (iov_numvfs_wanted >= 0) {
> +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_SRIOV;
> +			iov_cmd.cmd_param = iov_numvfs_wanted;
> +		} else if (iov_numnetdevs_wanted >= 0) {
> +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_NETDEVS;
> +			iov_cmd.cmd_param = iov_numnetdevs_wanted;
> +		} else if (iov_numvqueues_wanted >= 0) {
> +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_VF_QUEUES;
> +			iov_cmd.cmd_param = iov_numvqueues_wanted;
> +		} else {
> +			return -EINVAL;
> +		}
[...]

So what if the user specifies multiple keywords?

Also missing an update to the manual page.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html