Re: [PATCH v2 net 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow

Re: [PATCH v2 net 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow

[Deep Shah]

Thanks Simon. v3 drops the #if and casts to the type of tx.freq instead, so
the assignment still happens where tx.freq is 64-bit but long isn't.

I went with the cast rather than just dropping the #if, since dropping it on
its own would bring back the -Woverflow you flagged on v1.

I also ran the sashiko prompts over the series myself before sending. The
only other thing it turned up was the missing .gitignore entry for the test
binary, which v3 adds too.

https://lore.kernel.org/netdev/20260801222923.39017-1-deepshah146@gmail.com/

[PATCH v2 net 0/2] ptp: fix scaled_ppm_to_ppb() overflow bypassing the max_adj check

[Deep Shah]

ptp_clock_adjtime() validates an ADJ_FREQUENCY request by converting
tx->freq to ppb and comparing it against ops->max_adj. On 64-bit systems
that conversion can overflow s64 and wrap the result back into range, so
a crafted tx->freq bypasses the check and reaches ->adjfine() unclamped.

Patch 1 rejects the overflow in ptp_clock_adjtime().
Patch 2 adds a regression test that crafts struct timex.freq directly
(testptp's int-ppb path cannot express the value) and confirms it is
rejected with -ERANGE.

Changes in v2:
- patch 1: unchanged, added Reviewed-by from Vadim Fedorenko.
- patch 2 (all from Simon Horman's review):
  - cast fd to unsigned before the shift in FD_TO_CLOCKID (UBSan)
  - only set the 64-bit test value on 64-bit builds (32-bit -Woverflow)
  - save and restore the clock frequency
  - skip instead of fail on -EBUSY (free-running clock)

Link to v1:
https://lore.kernel.org/netdev/20260712040922.6403-1-deepshah146@gmail.com/

Deep Shah (2):
  ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()
  selftests: ptp: add a regression test for the frequency adjustment
    overflow

 drivers/ptp/ptp_clock.c                       | 14 ++-
 tools/testing/selftests/ptp/Makefile          |  2 +-
 .../testing/selftests/ptp/ptp_freq_overflow.c | 99 +++++++++++++++++++
 3 files changed, 113 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/ptp/ptp_freq_overflow.c


base-commit: 1c975de3343cdef506f2eecc833cc1f14b0401c4
-- 
2.43.0

[PATCH v2 net 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow

[Deep Shah]

testptp's -f option stores the requested adjustment as an int ppb and
converts it to scaled ppm, so it cannot express the 64-bit scaled-ppm
values needed to overflow scaled_ppm_to_ppb() and bypass the max_adj
check enforced by ptp_clock_adjtime().

Add a small test that crafts struct timex.freq directly and verifies that
an overflowing frequency adjustment is rejected with -ERANGE.  The test
skips when no frequency-adjustable PTP device is available.

Signed-off-by: Deep Shah <deepshah146@gmail.com>
---
 tools/testing/selftests/ptp/Makefile          |  2 +-
 .../testing/selftests/ptp/ptp_freq_overflow.c | 99 +++++++++++++++++++
 2 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/ptp/ptp_freq_overflow.c

diff --git a/tools/testing/selftests/ptp/Makefile b/tools/testing/selftests/ptp/Makefile
index 8f57f88ecadd..dd7376cc9bf5 100644
--- a/tools/testing/selftests/ptp/Makefile
+++ b/tools/testing/selftests/ptp/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 CFLAGS += $(KHDR_INCLUDES)
-TEST_GEN_PROGS := testptp
+TEST_GEN_PROGS := testptp ptp_freq_overflow
 LDLIBS += -lrt
 TEST_PROGS = phc.sh
 
diff --git a/tools/testing/selftests/ptp/ptp_freq_overflow.c b/tools/testing/selftests/ptp/ptp_freq_overflow.c
new file mode 100644
index 000000000000..417c0516622d
--- /dev/null
+++ b/tools/testing/selftests/ptp/ptp_freq_overflow.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Regression test for the scaled_ppm_to_ppb() integer overflow that allowed
+ * a crafted clock_adjtime(ADJ_FREQUENCY) to bypass the PTP max_adj check.
+ *
+ * testptp's -f option stores the adjustment as an int ppb and cannot express
+ * the 64-bit scaled-ppm values needed to overflow the conversion, so this
+ * test crafts struct timex.freq directly.
+ */
+#define _GNU_SOURCE
+#define __SANE_USERSPACE_TYPES__
+#include <errno.h>
+#include <fcntl.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/timex.h>
+#include <time.h>
+#include <unistd.h>
+#include <linux/ptp_clock.h>
+#include "../kselftest.h"
+
+#define FD_TO_CLOCKID(fd)	((clockid_t)((((unsigned int)~(fd)) << 3) | 3))
+
+/* clock_adjtime is not available in GLIBC < 2.14 */
+#if !__GLIBC_PREREQ(2, 14)
+#include <sys/syscall.h>
+static int clock_adjtime(clockid_t id, struct timex *tx)
+{
+	return syscall(__NR_clock_adjtime, id, tx);
+}
+#endif
+
+int main(int argc, char *argv[])
+{
+	const char *device = argc > 1 ? argv[1] : "/dev/ptp0";
+	struct ptp_clock_caps caps;
+	struct timex restore = { 0 };
+	struct timex tx = { 0 };
+	clockid_t clkid;
+	int fd, ret;
+
+	ksft_print_header();
+	ksft_set_plan(1);
+
+	if (sizeof(tx.freq) < 8)
+		ksft_exit_skip("the overflow only affects 64-bit kernels\n");
+
+	fd = open(device, O_RDWR);
+	if (fd < 0)
+		ksft_exit_skip("cannot open %s: %s\n", device, strerror(errno));
+
+	clkid = FD_TO_CLOCKID(fd);
+
+	if (ioctl(fd, PTP_CLOCK_GETCAPS, &caps))
+		ksft_exit_skip("PTP_CLOCK_GETCAPS on %s: %s\n", device, strerror(errno));
+	if (!caps.max_adj)
+		ksft_exit_skip("%s does not support frequency adjustment\n", device);
+
+	/*
+	 * Remember the current frequency.  A vulnerable kernel accepts the
+	 * bogus value below and programs it into the hardware, so restore the
+	 * original afterwards instead of leaving the clock corrupted.
+	 */
+	if (clock_adjtime(clkid, &restore))
+		ksft_exit_skip("clock_adjtime(get) on %s: %s\n", device, strerror(errno));
+	restore.modes = ADJ_FREQUENCY;
+
+	/*
+	 * (1 + 147573952589676412) * 125 == 2^64 + 9, which overflows s64 in
+	 * scaled_ppm_to_ppb() and wraps the result to a ppb of 0.  A kernel
+	 * that does not detect the overflow lets this absurd frequency past
+	 * the max_adj check; a fixed kernel rejects it with -ERANGE.
+	 */
+	tx.modes = ADJ_FREQUENCY;
+#if __SIZEOF_LONG__ >= 8
+	tx.freq = 147573952589676412L;
+#endif
+
+	ret = clock_adjtime(clkid, &tx);
+	if (ret < 0 && errno == EBUSY) {
+		/*
+		 * A free-running physical clock (virtual clocks active) rejects
+		 * frequency adjustment with -EBUSY before the overflow is even
+		 * evaluated, so the test cannot run here.
+		 */
+		ksft_test_result_skip("%s: frequency adjustment returned EBUSY, skipping\n",
+				      device);
+	} else {
+		ksft_test_result(ret < 0 && errno == ERANGE,
+				 "overflowing frequency adjustment is rejected (ret=%d errno=%d)\n",
+				 ret, ret < 0 ? errno : 0);
+	}
+
+	/* put the frequency back the way we found it */
+	clock_adjtime(clkid, &restore);
+
+	close(fd);
+	ksft_finished();
+}
-- 
2.43.0

Re: [PATCH v2 net 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow

[Simon Horman]

On Tue, Jul 21, 2026 at 01:42:56AM +0000, Deep Shah wrote:
> testptp's -f option stores the requested adjustment as an int ppb and
> converts it to scaled ppm, so it cannot express the 64-bit scaled-ppm
> values needed to overflow scaled_ppm_to_ppb() and bypass the max_adj
> check enforced by ptp_clock_adjtime().
> 
> Add a small test that crafts struct timex.freq directly and verifies that
> an overflowing frequency adjustment is rejected with -ERANGE.  The test
> skips when no frequency-adjustable PTP device is available.
> 
> Signed-off-by: Deep Shah <deepshah146@gmail.com>

...

> diff --git a/tools/testing/selftests/ptp/ptp_freq_overflow.c b/tools/testing/selftests/ptp/ptp_freq_overflow.c

...

> +	/*
> +	 * (1 + 147573952589676412) * 125 == 2^64 + 9, which overflows s64 in
> +	 * scaled_ppm_to_ppb() and wraps the result to a ppb of 0.  A kernel
> +	 * that does not detect the overflow lets this absurd frequency past
> +	 * the max_adj check; a fixed kernel rejects it with -ERANGE.
> +	 */
> +	tx.modes = ADJ_FREQUENCY;
> +#if __SIZEOF_LONG__ >= 8
> +	tx.freq = 147573952589676412L;
> +#endif

Thanks for the update here.
But unfortunately sashiko.dev still flags a problem.

  "Will this preprocessor condition cause a false test failure on 32-bit
   architectures configured with 64-bit time types (such as x32 or y2038
   compliant systems)?

  "On those architectures, the runtime check earlier in main() passes because
   sizeof(tx.freq) is 8. However, __SIZEOF_LONG__ evaluates to 4, which causes
   the preprocessor to skip the assignment of tx.freq.

  "This leaves tx.freq as 0, allowing clock_adjtime() to successfully apply
   a zero adjustment instead of rejecting the call with -ERANGE, which triggers
   a test failure.

I wonder if dropping the #if/#endif would address that concern.