netdev - Re: [PATCH V3 net-next 2/4] ethtool: extend coalesce setting uAPI with CQE mode

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]

Message-ID: <a366c691-fb81-8e30-3853-3260ceabf080@linux.ibm.com>
Date:   Tue, 14 Sep 2021 20:31:29 +0300
From:   Julian Wiedmann <jwi@...ux.ibm.com>
To:     Yufeng Mo <moyufeng@...wei.com>, davem@...emloft.net,
        kuba@...nel.org
Cc:     netdev@...r.kernel.org, shenjian15@...wei.com,
        lipeng321@...wei.com, yisen.zhuang@...wei.com,
        linyunsheng@...wei.com, huangguangbin2@...wei.com,
        chenhao288@...ilicon.com, salil.mehta@...wei.com,
        linuxarm@...wei.com, linuxarm@...neuler.org, dledford@...hat.com,
        jgg@...pe.ca, netanel@...zon.com, akiyano@...zon.com,
        thomas.lendacky@....com, irusskikh@...vell.com,
        michael.chan@...adcom.com, edwin.peer@...adcom.com,
        rohitm@...lsio.com, jacob.e.keller@...el.com,
        ioana.ciornei@....com, vladimir.oltean@....com,
        sgoutham@...vell.com, sbhatta@...vell.com, saeedm@...dia.com,
        ecree.xilinx@...il.com, grygorii.strashko@...com,
        merez@...eaurora.org, kvalo@...eaurora.org,
        linux-wireless@...r.kernel.org
Subject: Re: [PATCH V3 net-next 2/4] ethtool: extend coalesce setting uAPI
 with CQE mode

On 20.08.21 10:35, Yufeng Mo wrote:
> In order to support more coalesce parameters through netlink,
> add two new parameter kernel_coal and extack for .set_coalesce
> and .get_coalesce, then some extra info can return to user with
> the netlink API.
> 
> Signed-off-by: Yufeng Mo <moyufeng@...wei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@...wei.com>
> ---

[...]

> index 81fa36a..f2abc31 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -1619,12 +1619,14 @@ static noinline_for_stack int ethtool_get_coalesce(struct net_device *dev,
>  						   void __user *useraddr)
>  {
>  	struct ethtool_coalesce coalesce = { .cmd = ETHTOOL_GCOALESCE };
> +	struct kernel_ethtool_coalesce kernel_coalesce = {};
>  	int ret;
>  
>  	if (!dev->ethtool_ops->get_coalesce)
>  		return -EOPNOTSUPP;
>  
> -	ret = dev->ethtool_ops->get_coalesce(dev, &coalesce);
> +	ret = dev->ethtool_ops->get_coalesce(dev, &coalesce, &kernel_coalesce,
> +					     NULL);
>  	if (ret)
>  		return ret;
>  
> @@ -1691,19 +1693,26 @@ ethtool_set_coalesce_supported(struct net_device *dev,
>  static noinline_for_stack int ethtool_set_coalesce(struct net_device *dev,
>  						   void __user *useraddr)
>  {
> +	struct kernel_ethtool_coalesce kernel_coalesce = {};
>  	struct ethtool_coalesce coalesce;
>  	int ret;
>  
> -	if (!dev->ethtool_ops->set_coalesce)
> +	if (!dev->ethtool_ops->set_coalesce && !dev->ethtool_ops->get_coalesce)
>  		return -EOPNOTSUPP;
>  

This needs to be

	if (!set_coalesce || !get_coalesce)
		return -EOPNOTSUPP;

Otherwise you end up calling a NULL pointer below if just _one_ of the
callbacks is available.


> +	ret = dev->ethtool_ops->get_coalesce(dev, &coalesce, &kernel_coalesce,
> +					     NULL);
> +	if (ret)
> +		return ret;
> +
>  	if (copy_from_user(&coalesce, useraddr, sizeof(coalesce)))
>  		return -EFAULT;
>  
>  	if (!ethtool_set_coalesce_supported(dev, &coalesce))
>  		return -EOPNOTSUPP;
>  
> -	ret = dev->ethtool_ops->set_coalesce(dev, &coalesce);
> +	ret = dev->ethtool_ops->set_coalesce(dev, &coalesce, &kernel_coalesce,
> +					     NULL);
>  	if (!ret)
>  		ethtool_notify(dev, ETHTOOL_MSG_COALESCE_NTF, NULL);
>  	return ret;
>