From: Justin Stitt <justinstitt@google.com>
To: John Stultz <jstultz@google.com>,
Thomas Gleixner <tglx@linutronix.de>,
Stephen Boyd <sboyd@kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
Bill Wendling <morbo@google.com>
Cc: linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
Justin Stitt <justinstitt@google.com>
Subject: [PATCH] ntp: safeguard against time_constant overflow case
Date: Mon, 06 May 2024 22:01:49 +0000 [thread overview]
Message-ID: <20240506-b4-sio-ntp-c-v1-1-a01281aa01ba@google.com> (raw)
Using syzkaller with the recently reintroduced signed integer overflow
sanitizer produces this UBSAN report:
[ 46.809326] ------------[ cut here ]------------
[ 46.812882] UBSAN: signed-integer-overflow in ../kernel/time/ntp.c:738:18
[ 46.817676] 9223372036854775806 + 4 cannot be represented in type 'long'
[ 46.822346] CPU: 1 PID: 685 Comm: syz-executor.0 Not tainted 6.8.0-rc2-00036-g679ee73ec453 #2
[ 46.828270] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 46.834836] Call Trace:
[ 46.836625] <TASK>
[ 46.838147] dump_stack_lvl+0x93/0xd0
[ 46.840771] handle_overflow+0x171/0x1b0
[ 46.843516] __do_adjtimex+0x1236/0x1440
[ 46.846275] do_adjtimex+0x2be/0x740
[ 46.848864] __x64_sys_clock_adjtime+0x154/0x1d0
[ 46.852164] do_syscall_64+0xd7/0x1b0
[ 46.854783] ? arch_exit_to_user_mode_prepare+0x11/0x60
[ 46.858426] entry_SYSCALL_64_after_hwframe+0x6f/0x77
[ 46.861914] RIP: 0033:0x7fde90aaf539
[ 46.864500] Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 14 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 8
[ 46.877151] RSP: 002b:00007ffebfe63358 EFLAGS: 00000246 ORIG_RAX: 0000000000000131
[ 46.882279] RAX: ffffffffffffffda RBX: 00007fde90be3f80 RCX: 00007fde90aaf539
[ 46.887270] RDX: 0000000000000000 RSI: 0000000020000280 RDI: 0000000000000000
[ 46.892174] RBP: 00007fde90b0e496 R08: 0000000000000000 R09: 0000000000000000
[ 46.897061] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[ 46.902020] R13: 0000000000000095 R14: 00007fde90be3f80 R15: 00007fde90be3f80
[ 46.906946] </TASK>
[ 46.908537] ---[ end trace ]---
Historically, the signed integer overflow sanitizer did not work in the
kernel due to its interaction with `-fwrapv` but this has since been
changed [1] in the newest version of Clang; It being re-enabled in the
kernel with Commit 557f8c582a9ba8ab ("ubsan: Reintroduce signed overflow
sanitizer").
Nonetheless, let's slightly rework the logic surrounding time_constant
and how it is incremented such that we avoid unintentional wrap-around
(even though it is extremely unlikely to be hit in non-fuzzing scenarios).
[1]: https://github.com/llvm/llvm-project/pull/82432
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
kernel/time/ntp.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 406dccb79c2b..a9f039601968 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -65,6 +65,9 @@ static s64 time_offset;
/* pll time constant: */
static long time_constant = 2;
+/* pll time constant increment: */
+static long time_constant_inc = 4;
+
/* maximum error (usecs): */
static long time_maxerror = NTP_PHASE_LIMIT;
@@ -734,10 +737,10 @@ static inline void process_adjtimex_modes(const struct __kernel_timex *txc,
if (txc->modes & ADJ_TIMECONST) {
time_constant = txc->constant;
- if (!(time_status & STA_NANO))
- time_constant += 4;
- time_constant = min(time_constant, (long)MAXTC);
- time_constant = max(time_constant, 0l);
+ if (!(time_status & STA_NANO) &&
+ unlikely(LONG_MAX - time_constant_inc >= time_constant))
+ time_constant += time_constant_inc;
+ time_constant = clamp_t(long, time_constant, 0, MAXTC);
}
if (txc->modes & ADJ_TAI &&
---
base-commit: 0106679839f7c69632b3b9833c3268c316c0a9fc
change-id: 20240506-b4-sio-ntp-c-c227b02c65a3
Best regards,
--
Justin Stitt <justinstitt@google.com>
next reply other threads:[~2024-05-06 22:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 22:01 Justin Stitt [this message]
2024-05-07 6:02 ` [PATCH] ntp: safeguard against time_constant overflow case John Stultz
2024-05-07 22:03 ` Justin Stitt
2024-05-14 8:51 ` Thomas Gleixner
2024-05-14 9:17 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240506-b4-sio-ntp-c-v1-1-a01281aa01ba@google.com \
--to=justinstitt@google.com \
--cc=jstultz@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=morbo@google.com \
--cc=nathan@kernel.org \
--cc=sboyd@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).