Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()
@ 2026-08-01 22:29 Deep Shah
  2026-08-01 22:29 ` [PATCH net-next v3 1/2] " Deep Shah
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Deep Shah @ 2026-08-01 22:29 UTC (permalink / raw)
  To: netdev, Richard Cochran, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Eric Dumazet, Andrew Lunn
  Cc: linux-kernel, linux-kselftest, Shuah Khan, Vadim Fedorenko,
	Simon Horman, 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.

No real user space asks for such a frequency, so this is hardening rather
than a fix anyone is waiting on, and it is targeted at net-next with no
Fixes tag per Jakub's feedback on v2.

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 v3:
- retarget at net-next and drop the Fixes tag (Jakub Kicinski)
- patch 1: unchanged
- patch 2:
  - cast the test value to the type of tx.freq rather than guarding on
    __SIZEOF_LONG__, which skipped the assignment on x32 and other
    y2038 configurations and failed the test there (Simon Horman)
  - add the built binary to .gitignore

Changes in v2:
- patch 1: 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)
  - avoid a -Woverflow warning on 32-bit
  - 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/
Link to v2:
https://lore.kernel.org/netdev/20260721014256.1876-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/.gitignore        |   1 +
 tools/testing/selftests/ptp/Makefile          |   2 +-
 .../testing/selftests/ptp/ptp_freq_overflow.c | 101 ++++++++++++++++++
 4 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/ptp/ptp_freq_overflow.c


base-commit: 69963a0678a347d57c4ac8b16939dba216eb95ce
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next v3 1/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()
  2026-08-01 22:29 [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Deep Shah
@ 2026-08-01 22:29 ` Deep Shah
  2026-08-02 18:49   ` Richard Cochran
  2026-08-01 22:29 ` [PATCH net-next v3 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow Deep Shah
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Deep Shah @ 2026-08-01 22:29 UTC (permalink / raw)
  To: netdev, Richard Cochran, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Eric Dumazet, Andrew Lunn
  Cc: linux-kernel, linux-kselftest, Shuah Khan, Vadim Fedorenko,
	Simon Horman, Deep Shah

ptp_clock_adjtime() validates an ADJ_FREQUENCY request by converting the
requested scaled ppm to ppb and comparing it against ops->max_adj:

	long ppb = scaled_ppm_to_ppb(tx->freq);
	if (ppb > ops->max_adj || ppb < -ops->max_adj)
		return -ERANGE;

scaled_ppm_to_ppb() computes (1 + ppm) * 125 >> 13 in s64.  For a
sufficiently large tx->freq the multiplication overflows s64 and wraps,
so the resulting ppb can fall back within [-max_adj, max_adj] and pass
the check.  The unclamped tx->freq is then handed to ->adjfine(), where
drivers scale it again (e.g. scaled_ppm * 762939453125 in ptp_idt82p33)
and program a bogus frequency word.

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

The caller already has write access to the PHC, so this hardens the
max_adj sanity check rather than crossing a privilege boundary, and
well-behaved user space (e.g. ptp4l) never requests such values.  It is
a follow-up to commit 475b92f93216 ("ptp: improve max_adj check against
unreasonable values"), which handled the analogous s32 narrowing but not
this multiplication overflow.

Detect the overflow with check_*_overflow() and reject the request in
ptp_clock_adjtime() instead of acting on the wrapped value.

Signed-off-by: Deep Shah <deepshah146@gmail.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
 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..4111342d64f0 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 would
+		 * otherwise wrap the result back into the max_adj range.
+		 */
+		if (check_add_overflow((s64)tx->freq, (s64)1, &tmp) ||
+		    check_mul_overflow(tmp, (s64)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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v3 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow
  2026-08-01 22:29 [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Deep Shah
  2026-08-01 22:29 ` [PATCH net-next v3 1/2] " Deep Shah
@ 2026-08-01 22:29 ` Deep Shah
  2026-08-02 18:49   ` Richard Cochran
  2026-08-06  1:16 ` [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Jakub Kicinski
  2026-08-06  1:20 ` patchwork-bot+netdevbpf
  3 siblings, 1 reply; 7+ messages in thread
From: Deep Shah @ 2026-08-01 22:29 UTC (permalink / raw)
  To: netdev, Richard Cochran, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Eric Dumazet, Andrew Lunn
  Cc: linux-kernel, linux-kselftest, Shuah Khan, Vadim Fedorenko,
	Simon Horman, 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/.gitignore        |   1 +
 tools/testing/selftests/ptp/Makefile          |   2 +-
 .../testing/selftests/ptp/ptp_freq_overflow.c | 101 ++++++++++++++++++
 3 files changed, 103 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/ptp/ptp_freq_overflow.c

diff --git a/tools/testing/selftests/ptp/.gitignore b/tools/testing/selftests/ptp/.gitignore
index 534ca26eee48..e63194b44395 100644
--- a/tools/testing/selftests/ptp/.gitignore
+++ b/tools/testing/selftests/ptp/.gitignore
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 testptp
+ptp_freq_overflow
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..e90477175958
--- /dev/null
+++ b/tools/testing/selftests/ptp/ptp_freq_overflow.c
@@ -0,0 +1,101 @@
+// 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.
+	 *
+	 * The cast avoids a -Woverflow warning where tx.freq is 32-bit
+	 * without tying the value to the width of long, which differs from
+	 * the width of tx.freq on x32 and other y2038 configurations.
+	 */
+	tx.modes = ADJ_FREQUENCY;
+	tx.freq = (__typeof__(tx.freq))147573952589676412LL;
+
+	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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v3 1/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()
  2026-08-01 22:29 ` [PATCH net-next v3 1/2] " Deep Shah
@ 2026-08-02 18:49   ` Richard Cochran
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Cochran @ 2026-08-02 18:49 UTC (permalink / raw)
  To: Deep Shah
  Cc: netdev, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Eric Dumazet, Andrew Lunn, linux-kernel, linux-kselftest,
	Shuah Khan, Vadim Fedorenko, Simon Horman

On Sat, Aug 01, 2026 at 10:29:22PM +0000, Deep Shah wrote:
> ptp_clock_adjtime() validates an ADJ_FREQUENCY request by converting the
> requested scaled ppm to ppb and comparing it against ops->max_adj:
> 
> 	long ppb = scaled_ppm_to_ppb(tx->freq);
> 	if (ppb > ops->max_adj || ppb < -ops->max_adj)
> 		return -ERANGE;
> 
> scaled_ppm_to_ppb() computes (1 + ppm) * 125 >> 13 in s64.  For a
> sufficiently large tx->freq the multiplication overflows s64 and wraps,
> so the resulting ppb can fall back within [-max_adj, max_adj] and pass
> the check.  The unclamped tx->freq is then handed to ->adjfine(), where
> drivers scale it again (e.g. scaled_ppm * 762939453125 in ptp_idt82p33)
> and program a bogus frequency word.
> 
> For example tx->freq = 147573952589676412 makes (1 + ppm) * 125 equal
> 2^64 + 9, which wraps to ppb == 0 and is accepted.
> 
> The caller already has write access to the PHC, so this hardens the
> max_adj sanity check rather than crossing a privilege boundary, and
> well-behaved user space (e.g. ptp4l) never requests such values.  It is
> a follow-up to commit 475b92f93216 ("ptp: improve max_adj check against
> unreasonable values"), which handled the analogous s32 narrowing but not
> this multiplication overflow.
> 
> Detect the overflow with check_*_overflow() and reject the request in
> ptp_clock_adjtime() instead of acting on the wrapped value.
> 
> Signed-off-by: Deep Shah <deepshah146@gmail.com>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Acked-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v3 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow
  2026-08-01 22:29 ` [PATCH net-next v3 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow Deep Shah
@ 2026-08-02 18:49   ` Richard Cochran
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Cochran @ 2026-08-02 18:49 UTC (permalink / raw)
  To: Deep Shah
  Cc: netdev, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Eric Dumazet, Andrew Lunn, linux-kernel, linux-kselftest,
	Shuah Khan, Vadim Fedorenko, Simon Horman

On Sat, Aug 01, 2026 at 10:29:23PM +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>

Acked-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()
  2026-08-01 22:29 [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Deep Shah
  2026-08-01 22:29 ` [PATCH net-next v3 1/2] " Deep Shah
  2026-08-01 22:29 ` [PATCH net-next v3 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow Deep Shah
@ 2026-08-06  1:16 ` Jakub Kicinski
  2026-08-06  1:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2026-08-06  1:16 UTC (permalink / raw)
  To: Deep Shah
  Cc: netdev, Richard Cochran, David S . Miller, Paolo Abeni,
	Eric Dumazet, Andrew Lunn, linux-kernel, linux-kselftest,
	Shuah Khan, Vadim Fedorenko, Simon Horman

On Sat,  1 Aug 2026 22:29:21 +0000 Deep Shah wrote:
> - patch 2:
>   - cast the test value to the type of tx.freq rather than guarding on
>     __SIZEOF_LONG__, which skipped the assignment on x32 and other
>     y2038 configurations and failed the test there (Simon Horman)
>   - add the built binary to .gitignore

the test doesn't seem worth carrying in the tree TBH
If we really want this sort of unit test we should probably
integrate it with more with the net side (the PTP target doesn't get
run by netdev CI) and automatically create a netdevsim device which
has a SW PTP etc. Again, not sure it's worth for a trival overflow test
tho.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()
  2026-08-01 22:29 [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Deep Shah
                   ` (2 preceding siblings ...)
  2026-08-06  1:16 ` [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Jakub Kicinski
@ 2026-08-06  1:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-06  1:20 UTC (permalink / raw)
  To: Deep Shah
  Cc: netdev, richardcochran, davem, kuba, pabeni, edumazet,
	andrew+netdev, linux-kernel, linux-kselftest, shuah,
	vadim.fedorenko, horms

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat,  1 Aug 2026 22:29:21 +0000 you 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.
> 
> No real user space asks for such a frequency, so this is hardening rather
> than a fix anyone is waiting on, and it is targeted at net-next with no
> Fixes tag per Jakub's feedback on v2.
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb()
    https://git.kernel.org/netdev/net-next/c/504ef04e8674
  - [net-next,v3,2/2] selftests: ptp: add a regression test for the frequency adjustment overflow
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-06  1:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-01 22:29 [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Deep Shah
2026-08-01 22:29 ` [PATCH net-next v3 1/2] " Deep Shah
2026-08-02 18:49   ` Richard Cochran
2026-08-01 22:29 ` [PATCH net-next v3 2/2] selftests: ptp: add a regression test for the frequency adjustment overflow Deep Shah
2026-08-02 18:49   ` Richard Cochran
2026-08-06  1:16 ` [PATCH net-next v3 0/2] ptp: reject frequency adjustments that overflow scaled_ppm_to_ppb() Jakub Kicinski
2026-08-06  1:20 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox