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

[PATCH 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.

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 | 74 +++++++++++++++++++
 3 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/ptp/ptp_freq_overflow.c


base-commit: 3f1f755366687d051174739fb99f7d560202f60b
-- 
2.43.0

[PATCH net 1/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()

[Deep Shah]

ptp_clock_adjtime() converts tx->freq to ppb with scaled_ppm_to_ppb()
and rejects the request if it exceeds ops->max_adj.  On 64-bit systems
that conversion computes (1 + ppm) * 125 in s64, which can overflow for
a large tx->freq and wrap the result back into [-max_adj, max_adj].  The
check then passes and the original out-of-range value is handed to
->adjfine().

For example tx->freq = 147573952589676412 makes (1 + ppm) * 125 equal
2^64 + 9, which wraps to ppb == 0 and is accepted.

Reject the request with -ERANGE if either the addition or the
multiplication overflows.  This hardens the max_adj sanity check and is
not a security fix.  It follows up commit 475b92f93216 ("ptp: improve
max_adj check against unreasonable values"), which fixed the analogous
s32 narrowing but not this overflow.

Fixes: d39a743511cd ("ptp: validate the requested frequency adjustment.")
Signed-off-by: Deep Shah <deepshah146@gmail.com>
---
 drivers/ptp/ptp_clock.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index d6f54ccaf93b..f83aa44b0a74 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/posix-clock.h>
 #include <linux/pps_kernel.h>
 #include <linux/property.h>
@@ -159,7 +160,18 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx)
 		delta = ktime_to_ns(kt);
 		err = ops->adjtime(ops, delta);
 	} else if (tx->modes & ADJ_FREQUENCY) {
-		long ppb = scaled_ppm_to_ppb(tx->freq);
+		long ppb;
+		s64 tmp;
+
+		/*
+		 * scaled_ppm_to_ppb() multiplies (1 + freq) by 125 in s64;
+		 * reject a ->freq large enough to overflow that, which could
+		 * otherwise wrap the result back into the max_adj range.
+		 */
+		if (check_add_overflow(tx->freq, 1, &tmp) ||
+		    check_mul_overflow(tmp, 125, &tmp))
+			return -ERANGE;
+		ppb = scaled_ppm_to_ppb(tx->freq);
 		if (ppb > ops->max_adj || ppb < -ops->max_adj)
 			return -ERANGE;
 		err = ops->adjfine(ops, tx->freq);
-- 
2.43.0

[PATCH 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 | 74 +++++++++++++++++++
 2 files changed, 75 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..342b22689f67
--- /dev/null
+++ b/tools/testing/selftests/ptp/ptp_freq_overflow.c
@@ -0,0 +1,74 @@
+// 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)(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 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);
+
+	/*
+	 * (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;
+	tx.freq = 147573952589676412LL;
+
+	ret = clock_adjtime(clkid, &tx);
+	ksft_test_result(ret < 0 && errno == ERANGE,
+			 "overflowing frequency adjustment is rejected (ret=%d errno=%d)\n",
+			 ret, ret < 0 ? errno : 0);
+
+	close(fd);
+	ksft_finished();
+}
-- 
2.43.0

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

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
https://sashiko.dev/#/patchset/20260712040922.6403-3-deepshah146@gmail.com?part=2
---
--- Patch [2]: [PATCH net 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow ---
commit be5629a2612f961645c213deea88db61fa06f083
Author: Deep Shah <deepshah146@gmail.com>

selftests: ptp: add a regression test for the frequency adjustment overflow
    
This commit adds a regression test that crafts a struct timex.freq directly
to verify if an overflowing frequency adjustment is correctly rejected by
ptp_clock_adjtime() with -ERANGE.

> 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 0000000000000..342b22689f679
> --- /dev/null
> +++ b/tools/testing/selftests/ptp/ptp_freq_overflow.c
[ ... ]
> +
> +#define FD_TO_CLOCKID(fd)	((~(clockid_t)(fd) << 3) | 3)

[Severity: Medium]
Does this macro left-shift a negative signed integer?

Casting fd (a positive integer) to clockid_t (a signed 32-bit int), applying
bitwise NOT, and left-shifting it by 3 yields a negative integer. Left-shifting
a negative signed integer invokes undefined behavior in ISO C, which will
trigger a runtime error if tests are run with Undefined Behavior Sanitizer
(UBSan) enabled.

Could the inverted value be cast to an unsigned integer prior to the shift,
similar to how it is handled in testptp.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;
> +	tx.freq = 147573952589676412LL;

[Severity: Low]
Will this assignment cause a -Woverflow compiler warning on 32-bit
architectures?

On 32-bit platforms where tx.freq is a 32-bit long (without _TIME_BITS=64),
assigning this 64-bit literal exceeds the maximum value of the signed integer
type. Although a runtime check prevents execution, the compiler still parses
the assignment and emits a -Woverflow warning during the build.

> +
> +	ret = clock_adjtime(clkid, &tx);

[Severity: Medium]
Does this test permanently disrupt the hardware clock frequency on vulnerable
kernels without restoring it?

If the kernel does not have the overflow fix, clock_adjtime() will bypass
the max_adj check and push the corrupted frequency downstream to the driver
via ops->adjfine(). Because the test exits immediately after verifying the
error code, the original frequency is never restored, leaving time
synchronization corrupted until a reboot or manual reset.

Should the test read the initial dialed_frequency and restore it at the
end of the test?

> +	ksft_test_result(ret < 0 && errno == ERANGE,
> +			 "overflowing frequency adjustment is rejected (ret=%d errno=%d)\n",
> +			 ret, ret < 0 ? errno : 0);

[Severity: Medium]
Will this test incorrectly fail if the PTP clock is in free-running mode?

If virtual clocks are active on the selected PTP device, ptp_clock_freerun()
returns true and the kernel explicitly returns -EBUSY before evaluating the
frequency overflow logic. Since the test expects -ERANGE, receiving -EBUSY
will cause the KSFT assertion to fail.

Should -EBUSY be accounted for as a valid condition to gracefully skip
the test?

> +
> +	close(fd);
> +	ksft_finished();
> +}

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

[Deep Shah]

Thanks Simon. v2 addresses all four points:
https://lore.kernel.org/netdev/20260721014256.1876-1-deepshah146@gmail.com/

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

[Vadim Fedorenko]

On 12/07/2026 05:09, Deep Shah wrote:
> 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.
> 
> 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 | 74 +++++++++++++++++++
>   3 files changed, 88 insertions(+), 2 deletions(-)
>   create mode 100644 tools/testing/selftests/ptp/ptp_freq_overflow.c
> 
> 
> base-commit: 3f1f755366687d051174739fb99f7d560202f60b

For the series:
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>