[bug#76892,v1] gnu: ntp: Fix crash.
Commit Message
ntpd segfaults at boot so here is a patch to fix that.
From 3ebd9d493240978d5f2b14e55ed63754c43151c3 Mon Sep 17 00:00:00 2001
Message-ID: <3ebd9d493240978d5f2b14e55ed63754c43151c3.1741533506.git.jakob.kirsch@web.de>
From: Jakob Kirsch <jakob.kirsch@web.de>
Date: Sun, 9 Mar 2025 16:16:43 +0100
Subject: [PATCH v1] gnu: ntp: Fix crash.
* gnu/packages/ntp.scm (ntp): Add patch.
* gnu/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch: Add patch.
* gnu/local.mk (dist_patch_DATA): Register patch.
Change-Id: Ib3524c13fb2a1e6c70f8733cac3faeb427d00296
---
gnu/local.mk | 1 +
gnu/packages/ntp.scm | 94 +++++++++----------
...fix-dereferencing-the-wrong-variable.patch | 45 +++++++++
3 files changed, 92 insertions(+), 48 deletions(-)
create mode 100644 gnu/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch
base-commit: c4f297a664869a18126b66eb5209de1fcceb42d8
--
2.48.1
Comments
On Sun, Mar 09, 2025 at 04:19:51PM +0100, Jakob Kirsch via Guix-patches via wrote:
> ntpd segfaults at boot so here is a patch to fix that.
> From 3ebd9d493240978d5f2b14e55ed63754c43151c3 Mon Sep 17 00:00:00 2001
> Message-ID: <3ebd9d493240978d5f2b14e55ed63754c43151c3.1741533506.git.jakob.kirsch@web.de>
> From: Jakob Kirsch <jakob.kirsch@web.de>
> Date: Sun, 9 Mar 2025 16:16:43 +0100
> Subject: [PATCH v1] gnu: ntp: Fix crash.
>
> * gnu/packages/ntp.scm (ntp): Add patch.
> * gnu/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch: Add patch.
> * gnu/local.mk (dist_patch_DATA): Register patch.
Is the only change in this patch that we build ntp with the new patch
file? And the rest of it is changes to indentation? Or are there other
changes too?
Yes I only added the patch to fix the crash (it's a typo where ep should be ep2) and ran `guix style ntp` on it.
Hi Jakob, Leo,
Leo Famulari <leo@famulari.name> writes:
> On Sun, Mar 09, 2025 at 04:19:51PM +0100, Jakob Kirsch via
> Guix-patches via wrote:
>> ntpd segfaults at boot so here is a patch to fix that.
>
>> From 3ebd9d493240978d5f2b14e55ed63754c43151c3 Mon Sep 17
>> 00:00:00 2001
>> Message-ID:
>> <3ebd9d493240978d5f2b14e55ed63754c43151c3.1741533506.git.jakob.kirsch@web.de>
>> From: Jakob Kirsch <jakob.kirsch@web.de>
>> Date: Sun, 9 Mar 2025 16:16:43 +0100
>> Subject: [PATCH v1] gnu: ntp: Fix crash.
>>
>> * gnu/packages/ntp.scm (ntp): Add patch.
>> *
>> gnu/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch:
>> Add patch.
>> * gnu/local.mk (dist_patch_DATA): Register patch.
>
> Is the only change in this patch that we build ntp with the new
> patch
> file? And the rest of it is changes to indentation? Or are there
> other
> changes too?
Here’s the upstream bug(s):
https://bugs.ntp.org/show_bug.cgi?id=3928
https://bugs.ntp.org/show_bug.cgi?id=3968
I think this patch is fine, we might also consider rolling back to
4.2.8p17.
-- Ian
@@ -1908,6 +1908,7 @@ dist_patch_DATA = \
%D%/packages/patches/nss-getcwd-nonnull.patch \
%D%/packages/patches/nss-increase-test-timeout.patch \
%D%/packages/patches/nss-3.56-pkgconfig.patch \
+ %D%/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch \
%D%/packages/patches/nvi-assume-preserve-path.patch \
%D%/packages/patches/nvi-dbpagesize-binpower.patch \
%D%/packages/patches/nvi-db4.patch \
@@ -132,62 +132,60 @@ (define-public chrony
(define-public ntp
(package
- (name "ntp")
- (version "4.2.8p18")
- (source
+ (name "ntp")
+ (version "4.2.8p18")
+ (source
(origin
(method url-fetch)
(uri (list (string-append
"https://www.eecis.udel.edu/~ntp/ntp_spool/ntp4/ntp-"
- (version-major+minor version)
- "/ntp-" version ".tar.gz")
- (string-append
- "http://archive.ntp.org/ntp4/ntp-"
- (version-major+minor version)
- "/ntp-" version ".tar.gz")))
+ (version-major+minor version) "/ntp-" version ".tar.gz")
+ (string-append "http://archive.ntp.org/ntp4/ntp-"
+ (version-major+minor version) "/ntp-" version
+ ".tar.gz")))
(sha256
(base32 "1rb8yksqxjcsjvww9kwnw1242qzszwixh916jj254a8szgrwb16g"))
+ (patches (search-patches
+ "ntp-fix-dereferencing-the-wrong-variable.patch"))
(modules '((guix build utils)))
- (snippet
- '(begin
- ;; Remove the bundled copy of libevent, but we must keep
- ;; sntp/libevent/build-aux since configure.ac contains
- ;; AC_CONFIG_AUX_DIR([sntp/libevent/build-aux])
- (rename-file "sntp/libevent/build-aux"
- "sntp/libevent:build-aux")
- (delete-file-recursively "sntp/libevent")
- (mkdir "sntp/libevent")
- (rename-file "sntp/libevent:build-aux"
- "sntp/libevent/build-aux")))))
- (native-inputs (list which pkg-config))
- (inputs
- (cons* openssl
- libevent
- ;; Build with POSIX capabilities support on GNU/Linux. This allows
- ;; 'ntpd' to run as non-root (when invoked with '-u'.)
- (if (target-linux?)
- (list libcap)
- '())))
- (arguments
- (list
- ;; Pass "--with-yielding-select=yes" so that 'configure' knows whether
- ;; 'select' yields when using pthreads in a cross-compilation context.
- #:configure-flags
- #~(list "--with-yielding-select=yes")
- #:phases
- #~(modify-phases %standard-phases
- (add-after 'unpack 'disable-network-test
- (lambda _
- (substitute* "tests/libntp/Makefile.in"
- (("test-decodenetnum\\$\\(EXEEXT\\) ") "")))))))
- (build-system gnu-build-system)
- (synopsis "Real time clock synchronization system")
- (description "NTP is a system designed to synchronize the clocks of
+ (snippet '(begin
+ ;; Remove the bundled copy of libevent, but we must keep
+ ;; sntp/libevent/build-aux since configure.ac contains
+ ;; AC_CONFIG_AUX_DIR([sntp/libevent/build-aux])
+ (rename-file "sntp/libevent/build-aux"
+ "sntp/libevent:build-aux")
+ (delete-file-recursively "sntp/libevent")
+ (mkdir "sntp/libevent")
+ (rename-file "sntp/libevent:build-aux"
+ "sntp/libevent/build-aux")))))
+ (native-inputs (list which pkg-config))
+ (inputs (cons* openssl libevent
+ ;; Build with POSIX capabilities support on GNU/Linux. This allows
+ ;; 'ntpd' to run as non-root (when invoked with '-u'.)
+ (if (target-linux?)
+ (list libcap)
+ '())))
+ (arguments
+ (list
+ ;; Pass "--with-yielding-select=yes" so that 'configure' knows whether
+ ;; 'select' yields when using pthreads in a cross-compilation context.
+ #:configure-flags
+ #~(list "--with-yielding-select=yes")
+ #:phases
+ #~(modify-phases %standard-phases
+ (add-after 'unpack 'disable-network-test
+ (lambda _
+ (substitute* "tests/libntp/Makefile.in"
+ (("test-decodenetnum\\$\\(EXEEXT\\) ")
+ "")))))))
+ (build-system gnu-build-system)
+ (synopsis "Real time clock synchronization system")
+ (description "NTP is a system designed to synchronize the clocks of
computers over a network.")
- (license (l:x11-style
- "https://www.eecis.udel.edu/~mills/ntp/html/copyright.html"
- "A non-copyleft free licence from the University of Delaware"))
- (home-page "https://www.ntp.org")))
+ (license (l:x11-style
+ "https://www.eecis.udel.edu/~mills/ntp/html/copyright.html"
+ "A non-copyleft free licence from the University of Delaware"))
+ (home-page "https://www.ntp.org")))
(define-public openntpd
(package
new file mode 100644
@@ -0,0 +1,45 @@
+Subject: [PATCH] Fix dereferencing the wrong variable
+
+In line 1911 in ntp_io.c, the code calls `create_interface(port, ep2)` and saves
+the return value in the variable `ep`, which is then checked to not be `NULL` in
+the next line. In case `ep` is `NULL`, the code starting in line 1923 is
+executed. Keep in mind that `ep` is `NULL` in this branch. The error is logged
+in line 1928 and the address inside `ep` is converted using `stoa` by calling
+`stoa(&ep->sin)`. This would normally be fine since `socktoa` catches a `NULL`
+pointer in line 43 in socktoa.c but `&ep->sin` isn't `NULL` but 0x24 as the
+field isn't the first one in the `endpt` struct.
+
+This then causes a segmentation fault by dereferencing the pointer `0x24` in
+line 46 as the code tries to get the address family using `AF(sock)`.
+
+This only happens when ntpd cannot create an interface which seems to happen at
+boot time leading to 6 crashes on my machine on average.
+
+The issue is that someone accidentally typed `ep` instead of the correct `ep2`.
+
+This bug is being tracked as 3968 and 3928 upstream.
+---
+ ntpd/ntp_io.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/ntpd/ntp_io.c b/ntpd/ntp_io.c
+index 9d79fe4..0e761ff 100644
+--- a/ntpd/ntp_io.c
++++ b/ntpd/ntp_io.c
+@@ -1921,11 +1921,11 @@ update_interfaces(
+ }
+ else {
+ DPRINT_INTERFACE(3,
+- (ep, "updating ", " new - FAILED"));
++ (ep2, "updating ", " new - FAILED"));
+
+ msyslog(LOG_ERR,
+ "cannot bind address %s",
+- stoa(&ep->sin));
++ stoa(&ep2->sin));
+ }
+ free(ep2);
+ }
+--
+2.48.1
+