From: Juergen Perlinger Date: Sun, 11 Oct 2015 12:12:31 +0000 (+0200) Subject: [Bug 2939] reslist NULL pointer dereference X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ffd5989df3dcab6ca87e88b738c87390debcc7d1;p=thirdparty%2Fntp.git [Bug 2939] reslist NULL pointer dereference [Bug 2940] Stack exhaustion in recursive traversal of restriction list -- these two where fixed together -- bk: 561a522f17IeDpYe_tbKK8LZ58xQMw --- diff --git a/ChangeLog b/ChangeLog index b022ef6f6..57edbac7c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ --- +* [Bug 2939] reslist NULL pointer dereference + [Bug 2940] Stack exhaustion in recursive traversal of restriction list + These are actually fixed together. perlinger@ntp.org +--- (4.2.8p4-RC1) 2015/10/06 Released by Harlan Stenn * [Bug 2332] (reopened) Exercise thread cancellation once before dropping diff --git a/ntpd/ntp_request.c b/ntpd/ntp_request.c index 8ffebad32..59bee9457 100644 --- a/ntpd/ntp_request.c +++ b/ntpd/ntp_request.c @@ -81,8 +81,8 @@ static void do_unconf (sockaddr_u *, endpt *, struct req_pkt *); static void set_sys_flag (sockaddr_u *, endpt *, struct req_pkt *); static void clr_sys_flag (sockaddr_u *, endpt *, struct req_pkt *); static void setclr_flags (sockaddr_u *, endpt *, struct req_pkt *, u_long); -static void list_restrict4 (restrict_u *, struct info_restrict **); -static void list_restrict6 (restrict_u *, struct info_restrict **); +static void list_restrict4 (const restrict_u *, struct info_restrict **); +static void list_restrict6 (const restrict_u *, struct info_restrict **); static void list_restrict (sockaddr_u *, endpt *, struct req_pkt *); static void do_resaddflags (sockaddr_u *, endpt *, struct req_pkt *); static void do_ressubflags (sockaddr_u *, endpt *, struct req_pkt *); @@ -1197,7 +1197,7 @@ mem_stats( ms->hashcount[i] = (u_char) max((u_int)peer_hash_count[i], UCHAR_MAX); - more_pkt(); + (void) more_pkt(); flush_pkt(); } @@ -1285,7 +1285,7 @@ loop_info( li->compliance = htonl((u_int32)(tc_counter)); li->watchdog_timer = htonl((u_int32)(current_time - sys_epoch)); - more_pkt(); + (void) more_pkt(); flush_pkt(); } @@ -1571,56 +1571,143 @@ setclr_flags( req_ack(srcadr, inter, inpkt, INFO_OKAY); } +/* There have been some issues with the restrict list processing, + * ranging from problems with deep recursion (resulting in stack + * overflows) and overfull reply buffers. + * + * To avoid this trouble the list reversal is done iteratively using a + * scratch pad. + */ +typedef struct RestrictStack RestrictStackT; +struct RestrictStack { + RestrictStackT *link; + size_t fcnt; + const restrict_u *pres[63]; +}; + +static size_t +getStackSheetSize( + RestrictStackT *sp + ) +{ + if (sp) + return sizeof(sp->pres)/sizeof(sp->pres[0]); + return 0u; +} + +static int/*BOOL*/ +pushRestriction( + RestrictStackT **spp, + const restrict_u *ptr + ) +{ + RestrictStackT *sp; + + if (NULL == (sp = *spp) || 0 == sp->fcnt) { + /* need another sheet in the scratch pad */ + sp = emalloc(sizeof(*sp)); + sp->link = *spp; + sp->fcnt = getStackSheetSize(sp); + *spp = sp; + } + sp->pres[--sp->fcnt] = ptr; + return TRUE; +} + +static int/*BOOL*/ +popRestriction( + RestrictStackT **spp, + const restrict_u **opp + ) +{ + RestrictStackT *sp; + + if (NULL == (sp = *spp) || sp->fcnt >= getStackSheetSize(sp)) + return FALSE; + + *opp = sp->pres[sp->fcnt++]; + if (sp->fcnt >= getStackSheetSize(sp)) { + /* discard sheet from scratch pad */ + *spp = sp->link; + free(sp); + } + return TRUE; +} + +static void +flushRestrictionStack( + RestrictStackT **spp + ) +{ + RestrictStackT *sp; + + while (NULL != (sp = *spp)) { + *spp = sp->link; + free(sp); + } +} + /* - * list_restrict4 - recursive helper for list_restrict dumps IPv4 + * list_restrict4 - iterative helper for list_restrict dumps IPv4 * restriction list in reverse order. */ static void list_restrict4( - restrict_u * res, + const restrict_u * res, struct info_restrict ** ppir ) { + RestrictStackT * rpad; struct info_restrict * pir; - if (res->link != NULL) - list_restrict4(res->link, ppir); - pir = *ppir; - pir->addr = htonl(res->u.v4.addr); - if (client_v6_capable) - pir->v6_flag = 0; - pir->mask = htonl(res->u.v4.mask); - pir->count = htonl(res->count); - pir->flags = htons(res->flags); - pir->mflags = htons(res->mflags); - *ppir = (struct info_restrict *)more_pkt(); + for (rpad = NULL; res; res = res->link) + if (!pushRestriction(&rpad, res)) + break; + + while (pir && popRestriction(&rpad, &res)) { + pir->addr = htonl(res->u.v4.addr); + if (client_v6_capable) + pir->v6_flag = 0; + pir->mask = htonl(res->u.v4.mask); + pir->count = htonl(res->count); + pir->flags = htons(res->flags); + pir->mflags = htons(res->mflags); + pir = (struct info_restrict *)more_pkt(); + } + flushRestrictionStack(&rpad); + *ppir = pir; } - /* - * list_restrict6 - recursive helper for list_restrict dumps IPv6 + * list_restrict6 - iterative helper for list_restrict dumps IPv6 * restriction list in reverse order. */ static void list_restrict6( - restrict_u * res, + const restrict_u * res, struct info_restrict ** ppir ) { + RestrictStackT * rpad; struct info_restrict * pir; - if (res->link != NULL) - list_restrict6(res->link, ppir); - pir = *ppir; - pir->addr6 = res->u.v6.addr; - pir->mask6 = res->u.v6.mask; - pir->v6_flag = 1; - pir->count = htonl(res->count); - pir->flags = htons(res->flags); - pir->mflags = htons(res->mflags); - *ppir = (struct info_restrict *)more_pkt(); + for (rpad = NULL; res; res = res->link) + if (!pushRestriction(&rpad, res)) + break; + + while (pir && popRestriction(&rpad, &res)) { + pir->addr6 = res->u.v6.addr; + pir->mask6 = res->u.v6.mask; + pir->v6_flag = 1; + pir->count = htonl(res->count); + pir->flags = htons(res->flags); + pir->mflags = htons(res->mflags); + pir = (struct info_restrict *)more_pkt(); + } + flushRestrictionStack(&rpad); + *ppir = pir; } @@ -1644,8 +1731,7 @@ list_restrict( /* * The restriction lists are kept sorted in the reverse order * than they were originally. To preserve the output semantics, - * dump each list in reverse order. A recursive helper function - * achieves that. + * dump each list in reverse order. The workers take care of that. */ list_restrict4(restrictlist4, &ir); if (client_v6_capable) @@ -2002,11 +2088,11 @@ do_trustkey( u_long trust ) { - register u_long *kp; + register uint32_t *kp; register int items; items = INFO_NITEMS(inpkt->err_nitems); - kp = (u_long *)&inpkt->u; + kp = (uint32_t *)&inpkt->u; while (items-- > 0) { authtrust(*kp, trust); kp++; @@ -2085,7 +2171,7 @@ req_get_traps( it = (struct info_trap *)prepare_pkt(srcadr, inter, inpkt, v6sizeof(struct info_trap)); - for (i = 0, tr = ctl_traps; i < COUNTOF(ctl_traps); i++, tr++) { + for (i = 0, tr = ctl_traps; it && i < COUNTOF(ctl_traps); i++, tr++) { if (tr->tr_flags & TRAP_INUSE) { if (IS_IPV4(&tr->tr_addr)) { if (tr->tr_localaddr == any_interface) @@ -2401,7 +2487,7 @@ get_clock_info( ic = (struct info_clock *)prepare_pkt(srcadr, inter, inpkt, sizeof(struct info_clock)); - while (items-- > 0) { + while (items-- > 0 && ic) { NSRCADR(&addr) = *clkaddr++; if (!ISREFCLOCKADR(&addr) || NULL == findexistingpeer(&addr, NULL, NULL, -1, 0)) { @@ -2540,7 +2626,7 @@ get_clkbug_info( ic = (struct info_clkbug *)prepare_pkt(srcadr, inter, inpkt, sizeof(struct info_clkbug)); - while (items-- > 0) { + while (items-- > 0 && ic) { NSRCADR(&addr) = *clkaddr++; if (!ISREFCLOCKADR(&addr) || NULL == findexistingpeer(&addr, NULL, NULL, -1, 0)) { @@ -2588,13 +2674,15 @@ fill_info_if_stats(void *data, interface_info_t *interface_info) struct info_if_stats **ifsp = (struct info_if_stats **)data; struct info_if_stats *ifs = *ifsp; endpt *ep = interface_info->ep; + + if (NULL == ifs) + return; ZERO(*ifs); if (IS_IPV6(&ep->sin)) { - if (!client_v6_capable) { + if (!client_v6_capable) return; - } ifs->v6_flag = 1; ifs->unaddr.addr6 = SOCK_ADDR6(&ep->sin); ifs->unbcast.addr6 = SOCK_ADDR6(&ep->bcast);