[PATCH net-next] ethtool: ioctl: account for sopass diff in set_wol

[PATCH net-next] ethtool: ioctl: account for sopass diff in set_wol

[Justin Chen]

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

sopass won't be set if wolopt doesn't change. This means the following
will fail to set the correct sopass.
ethtool -s eth0 wol s sopass 11:22:33:44:55:66
ethtool -s eth0 wol s sopass 22:44:55:66:77:88

Make sure we call into the driver layer set_wol if sopass is different.

Fixes: 55b24334c0f2 ("ethtool: ioctl: improve error checking for set_wol")
Signed-off-by: Justin Chen <justin.chen@broadcom.com>
---

Note: Tagged "Fixes" patch has not hit rc yet.

 net/ethtool/ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 37b582225854..4a51e0ec295c 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1452,7 +1452,8 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr)
 	if (wol.wolopts & ~cur_wol.supported)
 		return -EINVAL;
 
-	if (wol.wolopts == cur_wol.wolopts)
+	if (wol.wolopts == cur_wol.wolopts &&
+	    !memcmp(wol.sopass, cur_wol.sopass, sizeof(wol.sopass)))
 		return 0;
 
 	ret = dev->ethtool_ops->set_wol(dev, &wol);
-- 
2.7.4


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4206 bytes --]

Re: [PATCH net-next] ethtool: ioctl: account for sopass diff in set_wol

[Andrew Lunn]

On Mon, Jun 12, 2023 at 02:37:00PM -0700, Justin Chen wrote:
> sopass won't be set if wolopt doesn't change. This means the following
> will fail to set the correct sopass.
> ethtool -s eth0 wol s sopass 11:22:33:44:55:66
> ethtool -s eth0 wol s sopass 22:44:55:66:77:88
> 
> Make sure we call into the driver layer set_wol if sopass is different.
> 
> Fixes: 55b24334c0f2 ("ethtool: ioctl: improve error checking for set_wol")
> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
> ---
> 
> Note: Tagged "Fixes" patch has not hit rc yet.
> 
>  net/ethtool/ioctl.c | 3 ++-

Hi Justin

Does the netlink version get this correct?

And just for my own curiosity, why are you so interested in the ioctl
version, which is deprecated and not used by modern versions of
ethtool?

	Andrew

Re: [PATCH net-next] ethtool: ioctl: account for sopass diff in set_wol

[Justin Chen]

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



On 6/12/23 3:28 PM, Andrew Lunn wrote:
> On Mon, Jun 12, 2023 at 02:37:00PM -0700, Justin Chen wrote:
>> sopass won't be set if wolopt doesn't change. This means the following
>> will fail to set the correct sopass.
>> ethtool -s eth0 wol s sopass 11:22:33:44:55:66
>> ethtool -s eth0 wol s sopass 22:44:55:66:77:88
>>
>> Make sure we call into the driver layer set_wol if sopass is different.
>>
>> Fixes: 55b24334c0f2 ("ethtool: ioctl: improve error checking for set_wol")
>> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
>> ---
>>
>> Note: Tagged "Fixes" patch has not hit rc yet.
>>
>>   net/ethtool/ioctl.c | 3 ++-
> 
> Hi Justin
> 
> Does the netlink version get this correct?
>

Yup netlink gets this correct

> And just for my own curiosity, why are you so interested in the ioctl
> version, which is deprecated and not used by modern versions of
> ethtool?
>
No particular reason. I was working on another driver and was planning 
to remove the set_wol error checking in the driver layer when I realized 
netlink set_wol was already doing it. Then on further inspection I 
noticed the ioctl version doesn't do the same thing.

Justin

> 	Andrew

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4206 bytes --]

Re: [PATCH net-next] ethtool: ioctl: account for sopass diff in set_wol

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 12 Jun 2023 14:37:00 -0700 you wrote:
> sopass won't be set if wolopt doesn't change. This means the following
> will fail to set the correct sopass.
> ethtool -s eth0 wol s sopass 11:22:33:44:55:66
> ethtool -s eth0 wol s sopass 22:44:55:66:77:88
> 
> Make sure we call into the driver layer set_wol if sopass is different.
> 
> [...]

Here is the summary with links:
  - [net-next] ethtool: ioctl: account for sopass diff in set_wol
    https://git.kernel.org/netdev/net-next/c/2bddad9ec65a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html