From 36565cd599cde3a35b26b5cb96aaea772042350f Mon Sep 17 00:00:00 2001 From: claudio Date: Thu, 11 Mar 2021 11:57:45 +0000 Subject: [PATCH] Revert rev 1.116 The repo structs are reallocated during runtime and so the back pointers to the head element of the TAILQ get corrupted. Noticed by tb@ --- usr.sbin/rpki-client/main.c | 80 ++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c index 6ae329340ad..0b59f4ebf4e 100644 --- a/usr.sbin/rpki-client/main.c +++ b/usr.sbin/rpki-client/main.c @@ -1,4 +1,4 @@ -/* $OpenBSD: main.c,v 1.117 2021/03/11 09:21:16 claudio Exp $ */ +/* $OpenBSD: main.c,v 1.118 2021/03/11 11:57:45 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -57,7 +57,6 @@ struct repo { char *local; /* local path name */ char *temp; /* temporary file / dir */ char *uris[REPO_MAX_URI]; /* URIs to fetch from */ - struct entityq queue; /* files waiting for this repo */ size_t id; /* identifier (array index) */ int uriidx; /* which URI is fetched */ int loaded; /* whether loaded or not */ @@ -212,13 +211,15 @@ entity_write_req(const struct entity *ent) * repo, then flush those into the parser process. */ static void -entityq_flush(struct repo *repo) +entityq_flush(struct entityq *q, const struct repo *repo) { struct entity *p, *np; - TAILQ_FOREACH_SAFE(p, &repo->queue, entries, np) { + TAILQ_FOREACH_SAFE(p, q, entries, np) { + if (p->repo < 0 || repo->id != (size_t)p->repo) + continue; entity_write_req(p); - TAILQ_REMOVE(&repo->queue, p, entries); + TAILQ_REMOVE(q, p, entries); entity_free(p); } } @@ -227,8 +228,9 @@ entityq_flush(struct repo *repo) * Add the heap-allocated file to the queue for processing. */ static void -entityq_add(char *file, enum rtype type, struct repo *rp, - const unsigned char *pkey, size_t pkeysz, char *descr) +entityq_add(struct entityq *q, char *file, enum rtype type, + const struct repo *rp, const unsigned char *pkey, size_t pkeysz, + char *descr) { struct entity *p; @@ -265,7 +267,7 @@ entityq_add(char *file, enum rtype type, struct repo *rp, entity_write_req(p); entity_free(p); } else - TAILQ_INSERT_TAIL(&rp->queue, p, entries); + TAILQ_INSERT_TAIL(q, p, entries); } /* @@ -283,7 +285,6 @@ repo_alloc(void) rp = &rt.repos[rt.reposz++]; rp->id = rt.reposz - 1; - TAILQ_INIT(&rp->queue); return rp; } @@ -395,7 +396,7 @@ repo_fetch(struct repo *rp) /* * Look up a trust anchor, queueing it for download if not found. */ -static struct repo * +static const struct repo * ta_lookup(const struct tal *tal) { struct repo *rp; @@ -429,7 +430,7 @@ ta_lookup(const struct tal *tal) /* * Look up a repository, queueing it for discovery if not found. */ -static struct repo * +static const struct repo * repo_lookup(const char *uri) { char *local, *repo; @@ -499,7 +500,8 @@ repo_filename(const struct repo *repo, const char *uri) * These are always relative to the directory in which "mft" sits. */ static void -queue_add_from_mft(const char *mft, const struct mftfile *file, enum rtype type) +queue_add_from_mft(struct entityq *q, const char *mft, + const struct mftfile *file, enum rtype type) { char *cp, *nfile; @@ -515,7 +517,7 @@ queue_add_from_mft(const char *mft, const struct mftfile *file, enum rtype type) * that the repository has already been loaded. */ - entityq_add(nfile, type, NULL, NULL, 0, NULL); + entityq_add(q, nfile, type, NULL, NULL, 0, NULL); } /* @@ -527,7 +529,7 @@ queue_add_from_mft(const char *mft, const struct mftfile *file, enum rtype type) * check the suffix anyway). */ static void -queue_add_from_mft_set(const struct mft *mft) +queue_add_from_mft_set(struct entityq *q, const struct mft *mft) { size_t i, sz; const struct mftfile *f; @@ -538,7 +540,7 @@ queue_add_from_mft_set(const struct mft *mft) assert(sz > 4); if (strcasecmp(f->file + sz - 4, ".crl") != 0) continue; - queue_add_from_mft(mft->file, f, RTYPE_CRL); + queue_add_from_mft(q, mft->file, f, RTYPE_CRL); } for (i = 0; i < mft->filesz; i++) { @@ -548,11 +550,11 @@ queue_add_from_mft_set(const struct mft *mft) if (strcasecmp(f->file + sz - 4, ".crl") == 0) continue; else if (strcasecmp(f->file + sz - 4, ".cer") == 0) - queue_add_from_mft(mft->file, f, RTYPE_CER); + queue_add_from_mft(q, mft->file, f, RTYPE_CER); else if (strcasecmp(f->file + sz - 4, ".roa") == 0) - queue_add_from_mft(mft->file, f, RTYPE_ROA); + queue_add_from_mft(q, mft->file, f, RTYPE_ROA); else if (strcasecmp(f->file + sz - 4, ".gbr") == 0) - queue_add_from_mft(mft->file, f, RTYPE_GBR); + queue_add_from_mft(q, mft->file, f, RTYPE_GBR); else logx("%s: unsupported file type: %s", mft->file, f->file); @@ -563,7 +565,7 @@ queue_add_from_mft_set(const struct mft *mft) * Add a local TAL file (RFC 7730) to the queue of files to fetch. */ static void -queue_add_tal(const char *file) +queue_add_tal(struct entityq *q, const char *file) { char *nfile, *buf; @@ -584,7 +586,7 @@ queue_add_tal(const char *file) } /* Not in a repository, so directly add to queue. */ - entityq_add(nfile, RTYPE_TAL, NULL, NULL, 0, buf); + entityq_add(q, nfile, RTYPE_TAL, NULL, NULL, 0, buf); /* entityq_add makes a copy of buf */ free(buf); } @@ -593,10 +595,10 @@ queue_add_tal(const char *file) * Add URIs (CER) from a TAL file, RFC 8630. */ static void -queue_add_from_tal(const struct tal *tal) +queue_add_from_tal(struct entityq *q, const struct tal *tal) { - char *nfile; - struct repo *repo; + char *nfile; + const struct repo *repo; assert(tal->urisz); @@ -604,7 +606,7 @@ queue_add_from_tal(const struct tal *tal) repo = ta_lookup(tal); nfile = ta_filename(repo, 0); - entityq_add(nfile, RTYPE_CER, repo, tal->pkey, + entityq_add(q, nfile, RTYPE_CER, repo, tal->pkey, tal->pkeysz, tal->descr); } @@ -612,10 +614,10 @@ queue_add_from_tal(const struct tal *tal) * Add a manifest (MFT) found in an X509 certificate, RFC 6487. */ static void -queue_add_from_cert(const struct cert *cert) +queue_add_from_cert(struct entityq *q, const struct cert *cert) { - struct repo *repo; - char *nfile; + const struct repo *repo; + char *nfile; repo = repo_lookup(cert->mft); if (repo == NULL) /* bad repository URI */ @@ -623,7 +625,7 @@ queue_add_from_cert(const struct cert *cert) nfile = repo_filename(repo, cert->mft); - entityq_add(nfile, RTYPE_MFT, repo, NULL, 0, NULL); + entityq_add(q, nfile, RTYPE_MFT, repo, NULL, 0, NULL); } /* @@ -633,7 +635,8 @@ queue_add_from_cert(const struct cert *cert) * In all cases, we gather statistics. */ static void -entity_process(int proc, struct stats *st, struct vrp_tree *tree) +entity_process(int proc, struct stats *st, struct entityq *q, + struct vrp_tree *tree) { enum rtype type; struct tal *tal; @@ -654,7 +657,7 @@ entity_process(int proc, struct stats *st, struct vrp_tree *tree) case RTYPE_TAL: st->tals++; tal = tal_read(proc); - queue_add_from_tal(tal); + queue_add_from_tal(q, tal); tal_free(tal); break; case RTYPE_CER: @@ -672,7 +675,7 @@ entity_process(int proc, struct stats *st, struct vrp_tree *tree) * we're revoked and then we don't want to * process the MFT. */ - queue_add_from_cert(cert); + queue_add_from_cert(q, cert); } else st->certs_invalid++; cert_free(cert); @@ -687,7 +690,7 @@ entity_process(int proc, struct stats *st, struct vrp_tree *tree) mft = mft_read(proc); if (mft->stale) st->mfts_stale++; - queue_add_from_mft_set(mft); + queue_add_from_mft_set(q, mft); mft_free(mft); break; case RTYPE_CRL: @@ -841,6 +844,7 @@ main(int argc, char *argv[]) size_t i, outsz = 0, talsz = 0; pid_t procpid, rsyncpid, httppid; int fd[2]; + struct entityq q; struct pollfd pfd[3]; struct roa **out = NULL; char *rsync_prog = "openrsync"; @@ -955,6 +959,8 @@ main(int argc, char *argv[]) if (talsz == 0) err(1, "no TAL files found in %s", "/etc/rpki"); + TAILQ_INIT(&q); + /* change working directory to the cache directory */ if (fchdir(cachefd) == -1) err(1, "fchdir"); @@ -1074,7 +1080,7 @@ main(int argc, char *argv[]) */ for (i = 0; i < talsz; i++) - queue_add_tal(tals[i]); + queue_add_tal(&q, tals[i]); while (entity_queue > 0 && !killme) { pfd[0].events = POLLIN; @@ -1150,7 +1156,7 @@ main(int argc, char *argv[]) "fallback to cache", rt.repos[i].local); rt.repos[i].loaded = 1; stats.repos++; - entityq_flush(&rt.repos[i]); + entityq_flush(&q, &rt.repos[i]); } if ((pfd[2].revents & POLLIN)) { @@ -1163,7 +1169,7 @@ main(int argc, char *argv[]) if (http_done(&rt.repos[i], ok)) { rt.repos[i].loaded = 1; stats.repos++; - entityq_flush(&rt.repos[i]); + entityq_flush(&q, &rt.repos[i]); } } @@ -1173,7 +1179,7 @@ main(int argc, char *argv[]) */ if ((pfd[1].revents & POLLIN)) { - entity_process(proc, &stats, &v); + entity_process(proc, &stats, &q, &v); } } @@ -1183,7 +1189,7 @@ main(int argc, char *argv[]) errx(1, "excessive runtime (%d seconds), giving up", timeout); } - assert(entity_queue == 0); + assert(TAILQ_EMPTY(&q)); logx("all files parsed: generating output"); rc = 0; -- 2.20.1