linux-kernel - [PATCH] ntp: Fixup adjtimex freq validation on 32bit systems

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Tue,  3 Feb 2015 10:57:38 -0800
From:	John Stultz <john.stultz@...aro.org>
To:	lkml <linux-kernel@...r.kernel.org>
Cc:	John Stultz <john.stultz@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>,
	Sasha Levin <sasha.levin@...cle.com>, stable@...r.kernel.org
Subject: [PATCH] ntp: Fixup adjtimex freq validation on 32bit systems

For tip/timers/urgent.

Additional validation of adjtimex freq values to avoid
potential multiplication overflows were added in commit
5e5aeb4367b (time: adjtimex: Validate the ADJ_FREQUENCY values)

Unfortunately the patch used LONG_MAX/MIN instead of
LLONG_MAX/MIN, which was fine on 64bit systems, but being
much smaller on 32bit systems caused false positives
resulting in most direct frequency adjustments to fail w/
EINVAL.

ntpd only does direct frequency adjustments at startup, so
the issue was not as easily observed there, but other time
sync applications like ptpd and chrony were more effected by
the bug.

See bugs:
https://bugzilla.kernel.org/show_bug.cgi?id=92481
https://bugzilla.redhat.com/show_bug.cgi?id=1188074

Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...nel.org>
Cc: Sasha Levin <sasha.levin@...cle.com>
Cc: stable@...r.kernel.org
Reported-by: Josh Boyer <jwboyer@...oraproject.org>
Reported-by: George Joseph <george.joseph@...rview5.com>
Tested-by: George Joseph <george.joseph@...rview5.com>
Signed-off-by: John Stultz <john.stultz@...aro.org>
---
One note:
0day kbuild bot complains about
>> kernel/time/ntp.c:637: warning: comparison is always false due to limited range of data type
>> kernel/time/ntp.c:639: warning: comparison is always false due to limited range of data type

We could fix this via adding an extra (BITS_PER_LONG == 64)
case before we check these to avoid it, but that seemed a
bit too ugly to me. Thoughts?

 kernel/time/ntp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 28bf91c..242774d 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -634,9 +634,9 @@ int ntp_validate_timex(struct timex *txc)
 		return -EPERM;
 
 	if (txc->modes & ADJ_FREQUENCY) {
-		if (LONG_MIN / PPM_SCALE > txc->freq)
+		if (LLONG_MIN / PPM_SCALE > txc->freq)
 			return -EINVAL;
-		if (LONG_MAX / PPM_SCALE < txc->freq)
+		if (LLONG_MAX / PPM_SCALE < txc->freq)
 			return -EINVAL;
 	}
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/