| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250817222933.21102-1-antoine@gagniere.dev>
Date: Mon, 18 Aug 2025 00:29:33 +0200
From: Antoine Gagniere <antoine@...niere.dev>
To: jonathan.lemon@...il.com,
vadim.fedorenko@...ux.dev
Cc: netdev@...r.kernel.org,
Antoine Gagniere <antoine@...niere.dev>
Subject: [PATCH] ptp: ocp: Fix PCI delay estimation
Since linux 6.12, a sign error causes the initial value of ts_window_adjust, (used in gettimex64) to be impossibly high, causing consumers like chrony to reject readings from PTP_SYS_OFFSET_EXTENDED.
This patch fixes ts_window_adjust's inital value and the sign-ness of various format flags
Context
-------
The value stored in the read-write attribute ts_window_adjust is a number of nanoseconds subtracted to the post_ts timestamp of the reading in gettimex64, used notably in the ioctl PTP_SYS_OFFSET_EXTENDED, to compensate for PCI delay.
Its initial value is set by estimating PCI delay.
Bug
---
The PCI delay estimation starts with the value U64_MAX and makes 3 measurements, taking the minimum value.
However because the delay was stored in a s64, U64_MAX was interpreted as -1, which compared as smaller than any positive values measured.
Then, that delay is divided by ~10 and placed in ts_window_adjust, which is a u32.
So ts_window_adjust ends up with (u32)(((s64)U64_MAX >> 5) * 3) inside, which is 4294967293
Symptom
-------
The consequence was that the post_ts of gettimex64, returned by PTP_SYS_OFFSET_EXTENDED, was substracted 4.29 seconds.
As a consequence chrony rejected all readings from the PHC
Difficulty to diagnose
----------------------
Using cat to read the attribute value showed -3 because the format flags %d was used instead of %u, resulting in a re-interpret cast.
Fixes
-----
1. Using U32_MAX as initial value for PCI delays: no one is expecting an ioread to take more than 4 s
This will correctly compare as bigger that actual PCI delay measurements.
2. Fixing the sign of various format flags
Signed-off-by: Antoine Gagniere <antoine@...niere.dev>
---
drivers/ptp/ptp_ocp.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index b651087f426f..153827722a63 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -1558,7 +1558,8 @@ ptp_ocp_watchdog(struct timer_list *t)
static void
ptp_ocp_estimate_pci_timing(struct ptp_ocp *bp)
{
- ktime_t start, end, delay = U64_MAX;
+ ktime_t start, end;
+ s64 delay_ns = U32_MAX; /* 4.29 seconds is high enough */
u32 ctrl;
int i;
@@ -1568,15 +1569,16 @@ ptp_ocp_estimate_pci_timing(struct ptp_ocp *bp)
iowrite32(ctrl, &bp->reg->ctrl);
- start = ktime_get_raw_ns();
+ start = ktime_get_raw();
ctrl = ioread32(&bp->reg->ctrl);
- end = ktime_get_raw_ns();
+ end = ktime_get_raw();
- delay = min(delay, end - start);
+ delay_ns = min(delay_ns, ktime_to_ns(end - start));
}
- bp->ts_window_adjust = (delay >> 5) * 3;
+ delay_ns = max(0, delay_ns);
+ bp->ts_window_adjust = (delay_ns >> 5) * 3;
}
static int
@@ -1894,7 +1896,7 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
int err;
fw_image = bp->fw_loader ? "loader" : "fw";
- sprintf(buf, "%d.%d", bp->fw_tag, bp->fw_version);
+ sprintf(buf, "%hhu.%hu", bp->fw_tag, bp->fw_version);
err = devlink_info_version_running_put(req, fw_image, buf);
if (err)
return err;
@@ -3196,7 +3198,7 @@ signal_show(struct device *dev, struct device_attribute *attr, char *buf)
i = (uintptr_t)ea->var;
signal = &bp->signal[i];
- count = sysfs_emit(buf, "%llu %d %llu %d", signal->period,
+ count = sysfs_emit(buf, "%lli %d %lli %d", signal->period,
signal->duty, signal->phase, signal->polarity);
ts = ktime_to_timespec64(signal->start);
@@ -3230,7 +3232,7 @@ period_show(struct device *dev, struct device_attribute *attr, char *buf)
struct ptp_ocp *bp = dev_get_drvdata(dev);
int i = (uintptr_t)ea->var;
- return sysfs_emit(buf, "%llu\n", bp->signal[i].period);
+ return sysfs_emit(buf, "%lli\n", bp->signal[i].period);
}
static EXT_ATTR_RO(signal, period, 0);
static EXT_ATTR_RO(signal, period, 1);
@@ -3244,7 +3246,7 @@ phase_show(struct device *dev, struct device_attribute *attr, char *buf)
struct ptp_ocp *bp = dev_get_drvdata(dev);
int i = (uintptr_t)ea->var;
- return sysfs_emit(buf, "%llu\n", bp->signal[i].phase);
+ return sysfs_emit(buf, "%lli\n", bp->signal[i].phase);
}
static EXT_ATTR_RO(signal, phase, 0);
static EXT_ATTR_RO(signal, phase, 1);
@@ -3289,7 +3291,7 @@ start_show(struct device *dev, struct device_attribute *attr, char *buf)
struct timespec64 ts;
ts = ktime_to_timespec64(bp->signal[i].start);
- return sysfs_emit(buf, "%llu.%lu\n", ts.tv_sec, ts.tv_nsec);
+ return sysfs_emit(buf, "%lli.%li\n", ts.tv_sec, ts.tv_nsec);
}
static EXT_ATTR_RO(signal, start, 0);
static EXT_ATTR_RO(signal, start, 1);
@@ -3444,7 +3446,7 @@ utc_tai_offset_show(struct device *dev,
{
struct ptp_ocp *bp = dev_get_drvdata(dev);
- return sysfs_emit(buf, "%d\n", bp->utc_tai_offset);
+ return sysfs_emit(buf, "%u\n", bp->utc_tai_offset);
}
static ssize_t
@@ -3472,7 +3474,7 @@ ts_window_adjust_show(struct device *dev,
{
struct ptp_ocp *bp = dev_get_drvdata(dev);
- return sysfs_emit(buf, "%d\n", bp->ts_window_adjust);
+ return sysfs_emit(buf, "%u\n", bp->ts_window_adjust);
}
static ssize_t
@@ -3964,7 +3966,7 @@ _signal_summary_show(struct seq_file *s, struct ptp_ocp *bp, int nr)
on = signal->running;
sprintf(label, "GEN%d", nr + 1);
- seq_printf(s, "%7s: %s, period:%llu duty:%d%% phase:%llu pol:%d",
+ seq_printf(s, "%7s: %s, period:%lli duty:%d%% phase:%lli pol:%d",
label, on ? " ON" : "OFF",
signal->period, signal->duty, signal->phase,
signal->polarity);
@@ -3974,7 +3976,7 @@ _signal_summary_show(struct seq_file *s, struct ptp_ocp *bp, int nr)
val = ioread32(®->status);
seq_printf(s, " %x]", val);
- seq_printf(s, " start:%llu\n", signal->start);
+ seq_printf(s, " start:%lli\n", signal->start);
}
static void
@@ -4231,7 +4233,7 @@ ptp_ocp_summary_show(struct seq_file *s, void *data)
seq_printf(s, "%7s: %lld.%ld == %ptT TAI\n", "PHC",
ts.tv_sec, ts.tv_nsec, &ts);
- seq_printf(s, "%7s: %lld.%ld == %ptT UTC offset %d\n", "SYS",
+ seq_printf(s, "%7s: %lld.%ld == %ptT UTC offset %u\n", "SYS",
sys_ts.tv_sec, sys_ts.tv_nsec, &sys_ts,
bp->utc_tai_offset);
seq_printf(s, "%7s: PHC:SYS offset: %lld window: %lld\n", "",
--
2.48.1
Powered by blists - more mailing lists