[net-next,6/7] net: dsa: make dsa_port_supports_hwtstamp() construct a fake ifreq

Commit Message

Vladimir Oltean April 2, 2023, 12:37 p.m. UTC

dsa_master_ioctl() is in the process of getting converted to a different
API, where we won't have access to a struct ifreq * anymore, but rather,
to a struct kernel_hwtstamp_config.

Since ds->ops->port_hwtstamp_get() still uses struct ifreq *, this
creates a difficult situation where we have to make up such a dummy
pointer.

The conversion is a bit messy, because it forces a "good" implementation
of ds->ops->port_hwtstamp_get() to return -EFAULT in copy_to_user()
because of the NULL ifr->ifr_data pointer. However, it works, and it is
only a transient step until ds->ops->port_hwtstamp_get() gets converted
to the new API which passes struct kernel_hwtstamp_config and does not
call copy_to_user().

Signed-off-by: Vladimir Oltean <[e-mail]>
---
 net/dsa/master.c |  2 +-
 net/dsa/port.c   | 10 ++++++----
 net/dsa/port.h   |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

Comments

Florian Fainelli April 2, 2023, 12:57 p.m. UTC | #1

On 4/2/2023 5:37 AM, Vladimir Oltean wrote:
> dsa_master_ioctl() is in the process of getting converted to a different
> API, where we won't have access to a struct ifreq * anymore, but rather,
> to a struct kernel_hwtstamp_config.
> 
> Since ds->ops->port_hwtstamp_get() still uses struct ifreq *, this
> creates a difficult situation where we have to make up such a dummy
> pointer.
> 
> The conversion is a bit messy, because it forces a "good" implementation
> of ds->ops->port_hwtstamp_get() to return -EFAULT in copy_to_user()
> because of the NULL ifr->ifr_data pointer. However, it works, and it is
> only a transient step until ds->ops->port_hwtstamp_get() gets converted
> to the new API which passes struct kernel_hwtstamp_config and does not
> call copy_to_user().
> 
> Signed-off-by: Vladimir Oltean <[e-mail]>

Reviewed-by: Florian Fainelli <[e-mail]>

Patch

diff --git a/net/dsa/master.c b/net/dsa/master.c
index 22d3f16b0e6d..e397641382ca 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -212,7 +212,7 @@  static int dsa_master_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		 * switch in the tree that is PTP capable.
 		 */
 		list_for_each_entry(dp, &dst->ports, list)
-			if (dsa_port_supports_hwtstamp(dp, ifr))
+			if (dsa_port_supports_hwtstamp(dp))
 				return -EBUSY;
 		break;
 	}
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 15cee17769e9..71ba30538411 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -114,19 +114,21 @@  static bool dsa_port_can_configure_learning(struct dsa_port *dp)
 	return !err;
 }
 
-bool dsa_port_supports_hwtstamp(struct dsa_port *dp, struct ifreq *ifr)
+bool dsa_port_supports_hwtstamp(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
+	struct ifreq ifr = {};
 	int err;
 
 	if (!ds->ops->port_hwtstamp_get || !ds->ops->port_hwtstamp_set)
 		return false;
 
 	/* "See through" shim implementations of the "get" method.
-	 * This will clobber the ifreq structure, but we will either return an
-	 * error, or the master will overwrite it with proper values.
+	 * Since we can't cook up a complete ioctl request structure, this will
+	 * fail in copy_to_user() with -EFAULT, which hopefully is enough to
+	 * detect a valid implementation.
 	 */
-	err = ds->ops->port_hwtstamp_get(ds, dp->index, ifr);
+	err = ds->ops->port_hwtstamp_get(ds, dp->index, &ifr);
 	return err != -EOPNOTSUPP;
 }
 
diff --git a/net/dsa/port.h b/net/dsa/port.h
index 9c218660d223..dc812512fd0e 100644
--- a/net/dsa/port.h
+++ b/net/dsa/port.h
@@ -15,7 +15,7 @@  struct switchdev_obj_port_mdb;
 struct switchdev_vlan_msti;
 struct phy_device;
 
-bool dsa_port_supports_hwtstamp(struct dsa_port *dp, struct ifreq *ifr);
+bool dsa_port_supports_hwtstamp(struct dsa_port *dp);
 void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
 			       const struct dsa_device_ops *tag_ops);
 int dsa_port_set_state(struct dsa_port *dp, u8 state, bool do_fast_age);