Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f7e23fe6-4d30-ef1b-a431-3ef6ec6f77ba@sangfor.com.cn>
Date:   Fri, 2 Jun 2023 23:01:43 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:102.0)
 Gecko/20100101 Thunderbird/102.10.0
Cc:     dinghui@sangfor.com.cn,
        Alexander H Duyck <alexander.duyck@gmail.com>,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, pengdonglin@sangfor.com.cn,
        huangcun@sangfor.com.cn
Subject: Re: [PATCH net-next] net: ethtool: Fix out-of-bounds copy to user
To:     Andrew Lunn <andrew@lunn.ch>
References: <20230601112839.13799-1-dinghui@sangfor.com.cn>
 <135a45b2c388fbaf9db4620cb01b95230709b9ac.camel@gmail.com>
 <eed0cbf7-ff12-057e-e133-0ddf5e98ef68@sangfor.com.cn>
 <6110cf9f-c10e-4b9b-934d-8d202b7f5794@lunn.ch>
Content-Language: en-US
From:   Ding Hui <dinghui@sangfor.com.cn>
In-Reply-To: <6110cf9f-c10e-4b9b-934d-8d202b7f5794@lunn.ch>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 2023/6/2 8:26 下午, Andrew Lunn wrote:
>>> Changing the copy size would not fix this. The problem is the driver
>>> will be overwriting with the size that it thinks it should be using.
>>> Reducing the value that is provided for the memory allocations will
>>> cause the driver to corrupt memory.
>>>
>>
>> I noticed that, in fact I did use the returned length to allocate
>> kernel memory, and only use adjusted length to copy to user.
> 
> This is also something i checked when quickly looking at the patch. It
> does look correct.
> 

Thanks.

> Also, RTNL should be held during the time both calls are made into the
> driver. So nothing from userspace should be able to get in the middle
> of these calls to change the number of queues.
> 

The RTNL lock is already be held during every each ioctl in dev_ethtool().

     rtnl_lock();
     rc = __dev_ethtool(net, ifr, useraddr, ethcmd, state);
     rtnl_unlock();

-- 
Thanks,
-dinghui