Subject: [bug#76892] [PATCH v1] gnu: ntp: Fix crash.

From: Jakob Kirsch <[email-masked]>
Date: Sun, 9 Mar 2025 16:19:51 +0100

ntpd segfaults at boot so here is a patch to fix that.
From 3ebd9d493240978d5f2b14e55ed63754c43151c3 Mon Sep 17 00:00:00 2001
Message-ID: <3ebd[email-masked]>
From: Jakob Kirsch <[email-masked]>
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

diff --git a/gnu/local.mk b/gnu/local.mk
index 9082ed04bf..9658a7fd08 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -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				\
diff --git a/gnu/packages/ntp.scm b/gnu/packages/ntp.scm
index 33ea790c9a..0b01f339d5 100644
--- a/gnu/packages/ntp.scm
+++ b/gnu/packages/ntp.scm
@@ -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
diff --git a/gnu/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch b/gnu/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch
new file mode 100644
index 0000000000..96108d2d83
--- /dev/null
+++ b/gnu/packages/patches/ntp-fix-dereferencing-the-wrong-variable.patch
@@ -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
+

base-commit: c4f297a664869a18126b66eb5209de1fcceb42d8
--
2.48.1