On Mon, Dec 18, 2023 at 07:35:07PM +0100, Otto Moerbeek wrote:

> On Mon, Dec 18, 2023 at 06:38:47PM +0100, Alexander Bluhm wrote:
> 
> > Hi,
> > 
> > for some days or weeks I see crashes of ntpd in accounting log on
> > my laptop.
> > 
> > Program terminated with signal SIGSEGV, Segmentation fault.
> > #0  log_sockaddr (sa=0x8) at /usr/src/usr.sbin/ntpd/util.c:159
> > 159             if (getnameinfo(sa, SA_LEN(sa), buf, sizeof(buf), NULL, 0,
> > (gdb) bt
> > #0  log_sockaddr (sa=0x8) at /usr/src/usr.sbin/ntpd/util.c:159
> > #1  0x00000b02fb57fc32 in constraint_msg_close (id=<optimized out>,
> >     data=0xb058f8f3770 "\001", len=4)
> >     at /usr/src/usr.sbin/ntpd/constraint.c:714
> > #2  0x00000b02fb575f8a in ntp_dispatch_imsg ()
> >     at /usr/src/usr.sbin/ntpd/ntp.c:516
> > #3  0x00000b02fb5758b8 in ntp_main (nconf=<optimized out>, pw=<optimized 
> > out>,
> >     argc=<optimized out>, argv=<optimized out>)
> >     at /usr/src/usr.sbin/ntpd/ntp.c:378
> > #4  0x00000b02fb57304a in main (argc=<optimized out>, argv=<optimized out>)
> >     at /usr/src/usr.sbin/ntpd/ntpd.c:224
> > 
> > (gdb) frame 1
> > #1  0x00000b02fb57fc32 in constraint_msg_close (id=<optimized out>,
> >     data=0xb058f8f3770 "\001", len=4)
> >     at /usr/src/usr.sbin/ntpd/constraint.c:714
> > 714                         log_sockaddr((struct sockaddr *)
> > (gdb) print cstr
> > $2 = (struct constraint *) 0xb05b96ac000
> > (gdb) print cstr->addr
> > $3 = (struct ntp_addr *) 0x0
> > 
> > Logging a null pointer address does not work.
> > 
> >    711          if (fail) {
> >    712                  log_debug("no constraint reply from %s"
> >    713                      " received in time, next query %ds",
> >    714                      log_sockaddr((struct sockaddr *)
> >    715                      &cstr->addr->ss), CONSTRAINT_SCAN_INTERVAL);
> > 
> > bluhm
> > 
> 
> This should prevent that and a few potenial similar cases.

Though this is a more fundamental appoach and is easier on the eyes imo.

        -Otto

Index: client.c
===================================================================
RCS file: /home/cvs/src/usr.sbin/ntpd/client.c,v
diff -u -p -r1.117 client.c
--- client.c    24 Mar 2022 07:37:19 -0000      1.117
+++ client.c    18 Dec 2023 19:05:41 -0000
@@ -351,8 +351,7 @@ client_dispatch(struct ntp_peer *p, u_in
                interval = error_interval();
                set_next(p, interval);
                log_info("reply from %s: not synced (%s), next query %llds",
-                   log_sockaddr((struct sockaddr *)&p->addr->ss), s,
-                       (long long)interval);
+                   log_ntp_addr(p->addr), s, (long long)interval);
                return (0);
        }
 
@@ -379,7 +378,7 @@ client_dispatch(struct ntp_peer *p, u_in
        if (!p->trusted && conf->constraint_median != 0 &&
            (constraint_check(T2) != 0 || constraint_check(T3) != 0)) {
                log_info("reply from %s: constraint check failed",
-                   log_sockaddr((struct sockaddr *)&p->addr->ss));
+                   log_ntp_addr(p->addr));
                set_next(p, error_interval());
                return (0);
        }
@@ -392,7 +391,7 @@ client_dispatch(struct ntp_peer *p, u_in
                set_next(p, interval);
                log_info("reply from %s: negative delay %fs, "
                    "next query %llds",
-                   log_sockaddr((struct sockaddr *)&p->addr->ss),
+                   log_ntp_addr(p->addr),
                    p->reply[p->shift].delay, (long long)interval);
                return (0);
        }
@@ -431,7 +430,7 @@ client_dispatch(struct ntp_peer *p, u_in
                if (p->trustlevel < TRUSTLEVEL_BADPEER &&
                    p->trustlevel + 1 >= TRUSTLEVEL_BADPEER)
                        log_info("peer %s now valid",
-                           log_sockaddr((struct sockaddr *)&p->addr->ss));
+                           log_ntp_addr(p->addr));
                p->trustlevel++;
        }
 
@@ -506,7 +505,7 @@ client_log_error(struct ntp_peer *peer, 
 {
        const char *address;
 
-       address = log_sockaddr((struct sockaddr *)&peer->addr->ss);
+       address = log_ntp_addr(peer->addr);
        if (peer->lasterror == error) {
                log_debug("%s %s: %s", operation, address, strerror(error));
                return;
Index: constraint.c
===================================================================
RCS file: /home/cvs/src/usr.sbin/ntpd/constraint.c,v
diff -u -p -r1.55 constraint.c
--- constraint.c        6 Dec 2023 15:51:53 -0000       1.55
+++ constraint.c        18 Dec 2023 19:02:25 -0000
@@ -469,8 +469,7 @@ priv_constraint_check_child(pid_t pid, i
                                    strsignal(sig) : "unknown";
                                log_warnx("constraint %s; "
                                    "terminated with signal %d (%s)",
-                                   log_sockaddr((struct sockaddr *)
-                                   &cstr->addr->ss), sig, signame);
+                                   log_ntp_addr(cstr->addr), sig, signame);
                        }
                        fail = 1;
                }
@@ -679,7 +678,7 @@ constraint_msg_result(u_int32_t id, u_in
            gettime_from_timeval(&tv[1]);
 
        log_info("constraint reply from %s: offset %f",
-           log_sockaddr((struct sockaddr *)&cstr->addr->ss),
+           log_ntp_addr(cstr->addr),
            offset);
 
        cstr->state = STATE_REPLY_RECEIVED;
@@ -711,8 +710,8 @@ constraint_msg_close(u_int32_t id, u_int
        if (fail) {
                log_debug("no constraint reply from %s"
                    " received in time, next query %ds",
-                   log_sockaddr((struct sockaddr *)
-                   &cstr->addr->ss), CONSTRAINT_SCAN_INTERVAL);
+                   log_ntp_addr(cstr->addr),
+                   CONSTRAINT_SCAN_INTERVAL);
                
                cnt = 0;
                TAILQ_FOREACH(tmp, &conf->constraints, entry)
Index: control.c
===================================================================
RCS file: /home/cvs/src/usr.sbin/ntpd/control.c,v
diff -u -p -r1.19 control.c
--- control.c   21 Apr 2021 09:38:11 -0000      1.19
+++ control.c   18 Dec 2023 19:02:43 -0000
@@ -347,7 +347,7 @@ build_show_peer(struct ctl_show_peer *cp
        now = getmonotime();
 
        if (p->addr) {
-               a = log_sockaddr((struct sockaddr *)&p->addr->ss);
+               a = log_ntp_addr(p->addr);
                if (p->addr->notauth)
                        auth = " (non-dnssec lookup)";
        }
Index: ntp.c
===================================================================
RCS file: /home/cvs/src/usr.sbin/ntpd/ntp.c,v
diff -u -p -r1.171 ntp.c
--- ntp.c       6 Dec 2023 15:51:53 -0000       1.171
+++ ntp.c       18 Dec 2023 19:05:24 -0000
@@ -261,13 +261,12 @@ ntp_main(struct ntpd_conf *nconf, struct
                        if (p->deadline > 0 && p->deadline <= getmonotime()) {
                                timeout = 300;
                                log_debug("no reply from %s received in time, "
-                                   "next query %ds", log_sockaddr(
-                                   (struct sockaddr *)&p->addr->ss), timeout);
+                                   "next query %ds", log_ntp_addr( p->addr),
+                                   timeout);
                                if (p->trustlevel >= TRUSTLEVEL_BADPEER &&
                                    (p->trustlevel /= 2) < TRUSTLEVEL_BADPEER)
                                        log_info("peer %s now invalid",
-                                           log_sockaddr(
-                                           (struct sockaddr *)&p->addr->ss));
+                                           log_ntp_addr(p->addr));
                                if (client_nextaddr(p) == 1) {
                                        peer_addr_head_clear(p);
                                        client_nextaddr(p);
@@ -276,8 +275,7 @@ ntp_main(struct ntpd_conf *nconf, struct
                        }
                        if (p->senderrors > MAX_SEND_ERRORS) {
                                log_debug("failed to send query to %s, "
-                                   "next query %ds", log_sockaddr(
-                                   (struct sockaddr *)&p->addr->ss),
+                                   "next query %ds", log_ntp_addr(p->addr),
                                    INTERVAL_QUERY_PATHETIC);
                                p->senderrors = 0;
                                if (client_nextaddr(p) == 1) {
@@ -419,16 +417,13 @@ ntp_main(struct ntpd_conf *nconf, struct
                                    conf->automatic)) {
                                case -1:
                                        log_debug("no reply from %s "
-                                           "received", log_sockaddr(
-                                           (struct sockaddr *) &pp->addr->ss));
+                                           "received", log_ntp_addr(pp->addr));
                                        if (pp->trustlevel >=
                                            TRUSTLEVEL_BADPEER &&
                                            (pp->trustlevel /= 2) <
                                            TRUSTLEVEL_BADPEER)
                                                log_info("peer %s now invalid",
-                                                   log_sockaddr(
-                                                   (struct sockaddr *)
-                                                   &pp->addr->ss));
+                                                   log_ntp_addr(pp->addr));
                                        break;
                                case 0: /* invalid replies are ignored */
                                        break;
@@ -634,8 +629,7 @@ ntp_dispatch_imsg_dns(void)
                                                continue;
                                        }
                                        log_debug("Adding address %s to %s",
-                                           log_sockaddr((struct sockaddr *)
-                                           &h->ss), peer->addr_head.name);
+                                           log_ntp_addr(h), 
peer->addr_head.name);
                                        npeer = new_peer();
                                        npeer->weight = peer->weight;
                                        npeer->query_addr4 = peer->query_addr4;
Index: ntpd.h
===================================================================
RCS file: /home/cvs/src/usr.sbin/ntpd/ntpd.h,v
diff -u -p -r1.152 ntpd.h
--- ntpd.h      27 Nov 2022 13:19:00 -0000      1.152
+++ ntpd.h      18 Dec 2023 18:53:11 -0000
@@ -407,6 +407,7 @@ double                       sfp_to_d(struct s_fixedpt);
 struct s_fixedpt        d_to_sfp(double);
 char                   *print_rtable(int);
 const char             *log_sockaddr(struct sockaddr *);
+const char             *log_ntp_addr(struct ntp_addr *);
 pid_t                   start_child(char *, int, int, char **);
 int                     sanitize_argv(int *, char ***);
 
Index: util.c
===================================================================
RCS file: /home/cvs/src/usr.sbin/ntpd/util.c,v
diff -u -p -r1.27 util.c
--- util.c      19 Nov 2023 10:41:25 -0000      1.27
+++ util.c      18 Dec 2023 19:06:00 -0000
@@ -163,6 +163,14 @@ log_sockaddr(struct sockaddr *sa)
                return (buf);
 }
 
+const char *
+log_ntp_addr(struct ntp_addr *addr)
+{
+       if (addr == NULL)
+               return ("(unknown)");
+       return log_sockaddr((struct sockaddr *)&addr->ss);
+}
+
 pid_t
 start_child(char *pname, int cfd, int argc, char **argv)
 {

Reply via email to