On 5/25/2026 7:30 AM, Sergey Temerkhanov wrote:
Use RCU to ensure the consistent state of the control PF global
pointer contained in struct ice_adapter. Enforce RCU usage on
the callers.
Fix a potential invalid pointer return due a TOCTOU issue
Fixes: e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825 devices")
No newline here; please keep all the tags together.
Signed-off-by: Sergey Temerkhanov <[email protected]>
Reviewed-by: Arkadiusz Kubalewski <[email protected]>
Tested-by: Frederick Lawler <[email protected]>
---
...
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 07e621813ff5..732964fd7c78 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -4,6 +4,7 @@
#include "ice.h"
#include "ice_lib.h"
#include "ice_trace.h"
+#include <linux/rcupdate.h>
I believe preference is to have kernel headers before the local ones.
static const char ice_pin_names[][64] = {
"SDP0",
@@ -54,11 +55,35 @@ static const struct ice_ptp_pin_desc ice_pin_desc_dpll[] = {
{ SDP3, { 3, -1 }, { 0, 0 }},
};
+/**
+ * ice_get_ctrl_pf - Get the control PF for a given PF
+ * @pf: The PF pointer to look up at
+ *
+ * The control PF is the PF which owns the PTP clock for the adapter.
+ * Only the control PF is allowed to perform certain operations on the
+ * PTP clock such as adjusting the time or configuring the pins.
+ *
+ * This function must be called from an RCU read-side critical section.
+ *
+ * Return: Pointer to the control PF, or NULL if not found
+ */
static struct ice_pf *ice_get_ctrl_pf(struct ice_pf *pf)
{
- return !pf->adapter ? NULL : pf->adapter->ctrl_pf;
+ return !pf->adapter ? NULL : rcu_dereference(pf->adapter->ctrl_pf);
}
+/**
+ * ice_get_ctrl_ptp - Get the PTP structure for the control PF
+ * @pf: The PF pointer to look up at
+ *
+ * The control PF is the PF which owns the PTP clock for the adapter.
+ * Only the control PF is allowed to perform certain operations on the
+ * PTP clock such as adjusting the time or configuring the pins.
+ *
+ * This function must be called from an RCU read-side critical section.
+ *
+ * Return: Pointer to the PTP structure of the control PF, or NULL if not found
+ */
static struct ice_ptp *ice_get_ctrl_ptp(struct ice_pf *pf)
{
struct ice_pf *ctrl_pf = ice_get_ctrl_pf(pf);
@@ -207,6 +232,8 @@ u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
u32 hi, lo, lo2;
u8 tmr_idx;
+ guard(rcu)();
This function is a bit big for guard()
"
Use of guard() is discouraged within any function longer than 20 lines,
scoped_guard() is considered more readable. Using normal lock/unlock is
still (weakly) preferred.
"
https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs
if (!ice_is_primary(hw))
hw = ice_get_primary_hw(pf);
@@ -3076,18 +3103,19 @@ void ice_ptp_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
static void ice_ptp_setup_adapter(struct ice_pf *pf)
{
- pf->adapter->ctrl_pf = pf;
+ rcu_assign_pointer(pf->adapter->ctrl_pf, pf);
}
static int ice_ptp_setup_pf(struct ice_pf *pf)
{
- struct ice_ptp *ctrl_ptp = ice_get_ctrl_ptp(pf);
struct ice_ptp *ptp = &pf->ptp;
- if (!ctrl_ptp) {
- dev_info(ice_pf_to_dev(pf),
- "PTP unavailable: no controlling PF\n");
- return -EOPNOTSUPP;
+ scoped_guard(rcu) {
+ if (!ice_get_ctrl_ptp(pf)) {
+ dev_info(ice_pf_to_dev(pf),
+ "PTP unavailable: no controlling PF\n");
+ return -EOPNOTSUPP;
+ }
}
if (pf->hw.mac_type == ICE_MAC_UNKNOWN)
@@ -3123,11 +3151,16 @@ static void ice_ptp_cleanup_pf(struct ice_pf *pf)
*/
int ice_ptp_clock_index(struct ice_pf *pf)
{
- struct ice_ptp *ctrl_ptp = ice_get_ctrl_ptp(pf);
+ struct ice_ptp *ctrl_ptp;
struct ptp_clock *clock;
+ guard(rcu)();
+
+ ctrl_ptp = ice_get_ctrl_ptp(pf);
+
No newline between the call and error check please.
Thanks,
Tony
if (!ctrl_ptp)
return -1;
+
clock = ctrl_ptp->clock;
return clock ? ptp_clock_index(clock) : -1;
