[PATCH net v3 1/2] net: ethtool: reject an out of range hwtstamp provider index

[PATCH net v3 1/2] net: ethtool: reject an out of range hwtstamp provider index

[Nicolai Buchwitz]

A provider index of 0xFFFFFFFF picks a provider on hardware that has
none. phc_index is an int where -1 means no PHC, and the u32 from user
space ends up as -1, so the two match.

TSINFO_GET uses -1 for "no provider requested" and answers with the
default provider instead of an error.

Reject the value in the netlink policy, which covers every comparison
site.

Fixes: b9e3f7dc9ed9 ("net: ethtool: tsinfo: Enhance tsinfo to support several hwtstamp by net topology")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
NLA_POLICY_MAX does not fit here, .max in struct nla_policy is s16.

 net/ethtool/ts.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ethtool/ts.h b/net/ethtool/ts.h
index d901a879a671..ab9805308353 100644
--- a/net/ethtool/ts.h
+++ b/net/ethtool/ts.h
@@ -5,9 +5,15 @@
 
 #include "netlink.h"
 
+/* phc_index is an int and -1 means no PHC, so keep the request non-negative */
+static const struct netlink_range_validation ethnl_ts_prov_index_range = {
+	.max = INT_MAX,
+};
+
 static const struct nla_policy
 ethnl_ts_hwtst_prov_policy[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_MAX + 1] = {
-	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] = { .type = NLA_U32 },
+	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] =
+		NLA_POLICY_FULL_RANGE(NLA_U32, &ethnl_ts_prov_index_range),
 	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_QUALIFIER] =
 		NLA_POLICY_MAX(NLA_U32, HWTSTAMP_PROVIDER_QUALIFIER_CNT - 1)
 };

base-commit: 11536ee3d3e0b1bd35b6f3f8df55a6053eb0c71d
-- 
2.53.0

[PATCH net v3 2/2] net: ethtool: let tsconfig reach a PHY-only timestamp provider

[Nicolai Buchwitz]

TSCONFIG_GET and TSCONFIG_SET reject a device that implements neither
hwtstamp NDO, even when its PHY can serve the request. The ioctls they
meant to replace handle it, so the two interfaces disagree on the same
hardware and user space has to pick one.

Accept the default timestamping PHY and an already installed provider on
both sides. On the set side the test moves into ethnl_set_tsconfig() as
the validate callback runs without rtnl. On the get side it stays ahead
of ethnl_ops_begin(), so a device that can serve nothing keeps failing
with EOPNOTSUPP and a dump still skips it rather than stopping there.

A netdev provider needs ndo_hwtstamp_set to be programmed at all, so
don't pick that source without it, and test for ndo_hwtstamp_get before
calling it.

Fixes: 6e9e2eed4f39 ("net: ethtool: Add support for tsconfig command to get/set hwtstamp config")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 net/core/dev_ioctl.c   |  3 ++-
 net/ethtool/tsconfig.c | 24 ++++++++++--------------
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 164643140a52..f6029f60c1dc 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -267,7 +267,8 @@ int dev_get_hwtstamp_phylib(struct net_device *dev,
 		    hwprov->phydev)
 			return phy_hwtstamp_get(hwprov->phydev, cfg);
 
-		if (hwprov->source == HWTSTAMP_SOURCE_NETDEV)
+		if (hwprov->source == HWTSTAMP_SOURCE_NETDEV &&
+		    dev->netdev_ops->ndo_hwtstamp_get)
 			return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
 
 		return -EOPNOTSUPP;
diff --git a/net/ethtool/tsconfig.c b/net/ethtool/tsconfig.c
index 6be3aa5d4bc1..2db0e7ba8b9f 100644
--- a/net/ethtool/tsconfig.c
+++ b/net/ethtool/tsconfig.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include <linux/net_tstamp.h>
+#include <linux/phy.h>
 #include <linux/ptp_clock_kernel.h>
 #include <net/netdev_lock.h>
 
@@ -42,7 +43,9 @@ static int tsconfig_prepare_data(const struct ethnl_req_info *req_base,
 	struct kernel_hwtstamp_config cfg = {};
 	int ret;
 
-	if (!dev->netdev_ops->ndo_hwtstamp_get)
+	if (!dev->netdev_ops->ndo_hwtstamp_get &&
+	    !phy_is_default_hwtstamp(dev->phydev) &&
+	    !netdev_ops_lock_dereference(dev->hwprov, dev))
 		return -EOPNOTSUPP;
 
 	ret = ethnl_ops_begin(dev);
@@ -248,17 +251,6 @@ static int tsconfig_send_reply(struct net_device *dev, struct genl_info *info)
 	return ret;
 }
 
-static int ethnl_set_tsconfig_validate(struct ethnl_req_info *req_base,
-				       struct genl_info *info)
-{
-	const struct net_device_ops *ops = req_base->dev->netdev_ops;
-
-	if (!ops->ndo_hwtstamp_set || !ops->ndo_hwtstamp_get)
-		return -EOPNOTSUPP;
-
-	return 1;
-}
-
 static struct hwtstamp_provider *
 tsconfig_set_hwprov_from_desc(struct net_device *dev,
 			      struct genl_info *info,
@@ -272,7 +264,7 @@ tsconfig_set_hwprov_from_desc(struct net_device *dev,
 	int ret;
 
 	ret = ethtool_net_get_ts_info_by_phc(dev, &ts_info, hwprov_desc);
-	if (!ret) {
+	if (!ret && dev->netdev_ops->ndo_hwtstamp_set) {
 		/* Found */
 		source = HWTSTAMP_SOURCE_NETDEV;
 	} else {
@@ -313,6 +305,11 @@ static int ethnl_set_tsconfig(struct ethnl_req_info *req_base,
 	if (!netif_device_present(dev))
 		return -ENODEV;
 
+	if (!dev->netdev_ops->ndo_hwtstamp_set &&
+	    !phy_is_default_hwtstamp(dev->phydev) &&
+	    !netdev_ops_lock_dereference(dev->hwprov, dev))
+		return -EOPNOTSUPP;
+
 	if (tb[ETHTOOL_A_TSCONFIG_HWTSTAMP_PROVIDER]) {
 		struct hwtstamp_provider_desc __hwprov_desc = {.index = -1};
 		struct hwtstamp_provider *__hwprov;
@@ -459,6 +456,5 @@ const struct ethnl_request_ops ethnl_tsconfig_request_ops = {
 	.reply_size		= tsconfig_reply_size,
 	.fill_reply		= tsconfig_fill_reply,
 
-	.set_validate		= ethnl_set_tsconfig_validate,
 	.set			= ethnl_set_tsconfig,
 };
-- 
2.53.0

Re: [PATCH net v3 1/2] net: ethtool: reject an out of range hwtstamp provider index

[Kory Maincent]

On 9/25/26 15:52, Nicolai Buchwitz wrote:
> A provider index of 0xFFFFFFFF picks a provider on hardware that has
> none. phc_index is an int where -1 means no PHC, and the u32 from user
> space ends up as -1, so the two match.
> 
> TSINFO_GET uses -1 for "no provider requested" and answers with the
> default provider instead of an error.
> 
> Reject the value in the netlink policy, which covers every comparison
> site.
> 
> Fixes: b9e3f7dc9ed9 ("net: ethtool: tsinfo: Enhance tsinfo to support several hwtstamp by net topology")
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> ---
> NLA_POLICY_MAX does not fit here, .max in struct nla_policy is s16.
> 
>   net/ethtool/ts.h | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ethtool/ts.h b/net/ethtool/ts.h
> index d901a879a671..ab9805308353 100644
> --- a/net/ethtool/ts.h
> +++ b/net/ethtool/ts.h
> @@ -5,9 +5,15 @@
>   
>   #include "netlink.h"
>   
> +/* phc_index is an int and -1 means no PHC, so keep the request non-negative */
> +static const struct netlink_range_validation ethnl_ts_prov_index_range = {
> +	.max = INT_MAX,
> +};
> +
>   static const struct nla_policy
>   ethnl_ts_hwtst_prov_policy[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_MAX + 1] = {
> -	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] = { .type = NLA_U32 },
> +	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] =
> +		NLA_POLICY_FULL_RANGE(NLA_U32, &ethnl_ts_prov_index_range),
>   	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_QUALIFIER] =
>   		NLA_POLICY_MAX(NLA_U32, HWTSTAMP_PROVIDER_QUALIFIER_CNT - 1)
>   };

I think you can simply do this:
[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] =
	NLA_POLICY_MAX(NLA_U32, INT_MAX),

> base-commit: 11536ee3d3e0b1bd35b6f3f8df55a6053eb0c71d

-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net v3 1/2] net: ethtool: reject an out of range hwtstamp provider index

[Nicolai Buchwitz]

Hi Kory

On 25.9.2026 15:55, Kory Maincent wrote:

> [...]

>>   static const struct nla_policy
>>   ethnl_ts_hwtst_prov_policy[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_MAX + 1] = 
>> {
>> -	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] = { .type = NLA_U32 },
>> +	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] =
>> +		NLA_POLICY_FULL_RANGE(NLA_U32, &ethnl_ts_prov_index_range),
>>   	[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_QUALIFIER] =
>>   		NLA_POLICY_MAX(NLA_U32, HWTSTAMP_PROVIDER_QUALIFIER_CNT - 1)
>>   };
> 
> I think you can simply do this:
> [ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] =
> 	NLA_POLICY_MAX(NLA_U32, INT_MAX),

That was my initial thought, but unfortunately it doesn't work. struct 
nla_policy's
.max is s16 and INT_MAX is truncated to -1. It ends up as U64_MAX in the 
range check
and in the end nothing gets rejected.

     include/vdso/limits.h:8:25: warning: overflow in conversion from 
'int'
     to 'short int' changes value from '2147483647' to '-1' [-Woverflow]

> [...]

Thanks,
Nicolai

Re: [PATCH net v3 1/2] net: ethtool: reject an out of range hwtstamp provider index

[Kory Maincent]

On 9/25/26 16:07, Nicolai Buchwitz wrote:
> Hi Kory
> 
> On 25.9.2026 15:55, Kory Maincent wrote:
> 
>> [...]
> 
>>>   static const struct nla_policy
>>>   ethnl_ts_hwtst_prov_policy[ETHTOOL_A_TS_HWTSTAMP_PROVIDER_MAX + 1] = {
>>> -    [ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] = { .type = NLA_U32 },
>>> +    [ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] =
>>> +        NLA_POLICY_FULL_RANGE(NLA_U32, &ethnl_ts_prov_index_range),
>>>       [ETHTOOL_A_TS_HWTSTAMP_PROVIDER_QUALIFIER] =
>>>           NLA_POLICY_MAX(NLA_U32, HWTSTAMP_PROVIDER_QUALIFIER_CNT - 1)
>>>   };
>>
>> I think you can simply do this:
>> [ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX] =
>>     NLA_POLICY_MAX(NLA_U32, INT_MAX),
> 
> That was my initial thought, but unfortunately it doesn't work. struct 
> nla_policy's
> .max is s16 and INT_MAX is truncated to -1. It ends up as U64_MAX in the 
> range check
> and in the end nothing gets rejected.
> 
>      include/vdso/limits.h:8:25: warning: overflow in conversion from 'int'
>      to 'short int' changes value from '2147483647' to '-1' [-Woverflow]

Indeed.
Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

Thank you!

-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net v3 2/2] net: ethtool: let tsconfig reach a PHY-only timestamp provider

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The removed ethnl_set_tsconfig_validate() required both
  ndo_hwtstamp_set and ndo_hwtstamp_get.

--

net: ethtool: let tsconfig reach a PHY-only timestamp provider

This patch lets TSCONFIG_GET and TSCONFIG_SET accept a device whose
timestamping is served by a default timestamping PHY or an already
installed hwprov, even when the netdev has no hwtstamp NDOs. It removes
the set_validate callback and moves the check into ethnl_set_tsconfig().
It also stops selecting a netdev source without ndo_hwtstamp_set, and
adds a NULL check for ndo_hwtstamp_get in dev_get_hwtstamp_phylib().

> diff --git a/net/ethtool/tsconfig.c b/net/ethtool/tsconfig.c
> index 6be3aa5d4bc1e..2db0e7ba8b9f5 100644
> --- a/net/ethtool/tsconfig.c
> +++ b/net/ethtool/tsconfig.c

[ ... ]

> @@ -42,7 +43,9 @@ static int tsconfig_prepare_data(const struct ethnl_req_info *req_base,
>  	struct kernel_hwtstamp_config cfg = {};
>  	int ret;
>  
> -	if (!dev->netdev_ops->ndo_hwtstamp_get)
> +	if (!dev->netdev_ops->ndo_hwtstamp_get &&
> +	    !phy_is_default_hwtstamp(dev->phydev) &&
> +	    !netdev_ops_lock_dereference(dev->hwprov, dev))
>  		return -EOPNOTSUPP;
>  
>  	ret = ethnl_ops_begin(dev);
> @@ -248,17 +251,6 @@ static int tsconfig_send_reply(struct net_device *dev, struct genl_info *info)
>  	return ret;
>  }
>  
> -static int ethnl_set_tsconfig_validate(struct ethnl_req_info *req_base,
> -				       struct genl_info *info)
> -{
> -	const struct net_device_ops *ops = req_base->dev->netdev_ops;
> -
> -	if (!ops->ndo_hwtstamp_set || !ops->ndo_hwtstamp_get)
> -		return -EOPNOTSUPP;
> -
> -	return 1;
> -}
> -

[ ... ]

> @@ -313,6 +305,11 @@ static int ethnl_set_tsconfig(struct ethnl_req_info *req_base,
>  	if (!netif_device_present(dev))
>  		return -ENODEV;
>  
> +	if (!dev->netdev_ops->ndo_hwtstamp_set &&
> +	    !phy_is_default_hwtstamp(dev->phydev) &&
> +	    !netdev_ops_lock_dereference(dev->hwprov, dev))
> +		return -EOPNOTSUPP;
> +

[Severity: Medium]

The removed ethnl_set_tsconfig_validate() required both ndo_hwtstamp_set
and ndo_hwtstamp_get. This new check in ethnl_set_tsconfig() only tests
ndo_hwtstamp_set. The matching check in tsconfig_prepare_data() only
tests ndo_hwtstamp_get.

What happens on a netdev that has ndo_hwtstamp_set but no
ndo_hwtstamp_get, and has neither a default timestamping PHY nor a
hwprov?

TSCONFIG_SET now accepts that device. However, reading the current
config fails in dev_get_hwtstamp_phylib():

	if (!dev->netdev_ops->ndo_hwtstamp_get)
		return -EOPNOTSUPP;

and ethnl_set_tsconfig() ignores that error:

		ret = dev_get_hwtstamp_phylib(dev, &hwtst_config);
		if (ret < 0 && ret != -EOPNOTSUPP)
			goto err_free_hwprov;

This means the read-modify-write starts from an all-zero hwtst_config.

A request that carries only RX_FILTERS or only FLAGS would then reset
tx_type to HWTSTAMP_TX_OFF and zero the other fields.

A full disable request (TX_OFF, FILTER_NONE, no flags) matches the
assumed zero state, so ethnl_bitmap32_update() never sets *mod. As a
result, config_mod stays false and dev_set_hwtstamp_phylib() is
skipped. Can hardware timestamping stay enabled while the command
reports success?

After that, tsconfig_send_reply()->tsconfig_prepare_data() fails its
own check with -EOPNOTSUPP, and ethnl_set_tsconfig() swallows it:

	ret = tsconfig_send_reply(dev, info);
	if (ret && ret != -EOPNOTSUPP) {

So the ETHTOOL_MSG_TSCONFIG_SET_REPLY documented in
Documentation/netlink/specs/ethtool.yaml is never sent. TSCONFIG_GET on
the same device still returns EOPNOTSUPP even though SET succeeds.

Every in-tree driver seems to implement both NDOs today. Elsewhere,
though, the patch treats set-without-get as a valid state: it selects
the NETDEV source in tsconfig_set_hwprov_from_desc() based on
ndo_hwtstamp_set alone, and it adds a NULL check for ndo_hwtstamp_get
in dev_get_hwtstamp_phylib().

For the netdev case, should the SET check also require
ndo_hwtstamp_get, so that GET and SET accept the same devices?

>  	if (tb[ETHTOOL_A_TSCONFIG_HWTSTAMP_PROVIDER]) {
>  		struct hwtstamp_provider_desc __hwprov_desc = {.index = -1};
>  		struct hwtstamp_provider *__hwprov;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925135237.3432266-2-nb%40tipi-net.de