]> git.ipfire.org Git - thirdparty/FORT-validator.git/commitdiff
Log: Revert ad841d5024bea7f7d9243a0aae5fdecc40afcd3b
authorAlberto Leiva Popper <ydahhrk@gmail.com>
Fri, 11 Jun 2021 19:59:29 +0000 (14:59 -0500)
committerAlberto Leiva Popper <ydahhrk@gmail.com>
Fri, 11 Jun 2021 20:42:30 +0000 (15:42 -0500)
ad841d50 was a mistake. It was never agreed in #40 that Fort should
shotgun blast its own face on the first ENOMEM, and even if it was, the
idea is preposterous. Memory allocation failures are neither programming
errors nor an excuse to leave all the routers hanging.

While there's some truth to the notion that a Fort memory leak (which
has been exhausting memory over time) could be temporarily amended by
killing Fort (and letting the OS clean it up), the argument completely
misses the reality that memory allocation failures could happen
regardless of the existence of a memory leak.

A memory leak is a valid reason to throw away the results of a current
validation run (as long as the admin is warned), but an existing
validation result and the RTR server must remain afloat.

Also includes a pr_enomem() caller review.

12 files changed:
src/asn1/decode.c
src/common.c
src/common.h
src/config.c
src/log.c
src/main.c
src/rrdp/rrdp_objects.c
src/rsync/rsync.c
src/rtr/pdu_handler.c
src/rtr/primitive_reader.c
src/slurm/db_slurm.c
src/uri.c

index 4e9b24372d27792374fb6021852a9cf8df62ef89..f4703d1d1b13819720b67aa5a506183f5f9b55a9 100644 (file)
@@ -96,9 +96,6 @@ asn1_decode(const void *buffer, size_t buffer_size,
        rval = ber_decode(&s_codec_ctx, descriptor, result, buffer,
            buffer_size);
        if (rval.code != RC_OK) {
-               if (rval.code == RC_FAIL && errno == ENOMEM)
-                       pr_enomem();
-
                /* Must free partial object according to API contracts. */
                ASN_STRUCT_FREE(*descriptor, *result);
                /* We expect the data to be complete; RC_WMORE is an error. */
index 83b6fe67efb640d412e3e9c8839f32422d9733b9..f49bb7442a49ae1fc982e5761289b0f6c6b55f81 100644 (file)
@@ -68,22 +68,6 @@ rwlock_unlock(pthread_rwlock_t *lock)
                    error);
 }
 
-void
-close_thread(pthread_t thread, char const *what)
-{
-       int error;
-
-       error = pthread_cancel(thread);
-       if (error && error != ESRCH)
-               pr_crit("pthread_cancel() threw %d on the '%s' thread.",
-                   error, what);
-
-       error = pthread_join(thread, NULL);
-       if (error)
-               pr_crit("pthread_join() threw %d on the '%s' thread.",
-                   error, what);
-}
-
 static int
 process_file(char const *dir_name, char const *file_name, char const *file_ext,
     int *fcount, process_file_cb cb, void *arg)
index 24fd976a0933b7d1cf79fbd5dfe2b99d8e3b65a3..1c92c41e9e2f14610e1459f9391292d96efa5c9f 100644 (file)
@@ -47,9 +47,6 @@ int rwlock_read_lock(pthread_rwlock_t *);
 void rwlock_write_lock(pthread_rwlock_t *);
 void rwlock_unlock(pthread_rwlock_t *);
 
-/** Also boilerplate. */
-void close_thread(pthread_t thread, char const *);
-
 typedef int (*process_file_cb)(char const *, void *);
 int process_file_or_dir(char const *, char const *, bool, process_file_cb,
     void *);
index 5194e288c12625839d526bd750968bfff2ce031b..adbe86188c0a6eb4023295aa44d14d6cb32bdd94 100644 (file)
@@ -964,7 +964,7 @@ set_default_values(void)
 
        error = string_array_init(&rpki_config.server.address, NULL, 0);
        if (error)
-               return pr_enomem();
+               return error;
 
        rpki_config.server.port = strdup("323");
        if (rpki_config.server.port == NULL) {
index d2c3d2432eb159a3f76844ec7872a3f724f4639d..9e8a65be0046003f8acff1e0f4cfb643900828d4 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -546,8 +546,27 @@ val_crypto_err(const char *format, ...)
 int
 pr_enomem(void)
 {
-       /* TODO this being a critical is not acceptable */
-       pr_crit("Out of memory.");
+       /*
+        * I'm not using PR_SIMPLE and friends, because those allocate.
+        * We want to minimize allocations after a memory allocation failure.
+        */
+
+       if (LOG_ERR > op_config.level)
+               return -ENOMEM;
+
+       if (op_config.syslog_enabled) {
+               lock_mutex();
+               syslog(LOG_ERR | op_config.facility, "Out of memory.");
+               unlock_mutex();
+       }
+
+       if (op_config.fprintf_enabled) {
+               lock_mutex();
+               fprintf(stderr, "Out of memory.\n");
+               unlock_mutex();
+       }
+
+       return -ENOMEM;
 }
 
 __dead void
index c85c7bb6681a6d42a0a1aa26b123f62f1be924b4..0d845d5ddac53859340f304fce6a6cbf1532c8e3 100644 (file)
@@ -34,12 +34,15 @@ main(int argc, char **argv)
 {
        int error;
 
-       printf("Fort 1.5.0.1\n");
        /* Initializations */
 
        error = log_setup();
        if (error)
-               return error;
+               goto just_quit;
+
+       /* TODO (issue49) don't forget to improve this. */
+       pr_op_info("Fort 1.5.0.2");
+
        error = thvar_init();
        if (error)
                goto revert_log;
@@ -115,5 +118,6 @@ revert_config:
 revert_log:
        log_teardown();
        PR_DEBUG_MSG("Main thread returning.");
-       return error;
+just_quit:
+       return abs(error);
 }
index 24807ba26f65e581da70fa77f7c00a117cb5b50e..b8d1c0568bd2cb307f0658a41ab258c7e9dd0075 100644 (file)
@@ -246,7 +246,7 @@ update_notification_create(struct update_notification **file)
        error = deltas_head_create(&list);
        if (error) {
                free(tmp);
-               return pr_enomem();
+               return error;
        }
        tmp->deltas_list = list;
        tmp->uri = NULL;
index 45b9ea1676a36ebfe63fd1fb712e68c33d78c714..f5129a611b02a3a3bdde9b778bc56fbada2c0d70 100644 (file)
@@ -248,7 +248,7 @@ prepare_rsync(struct rpki_uri *uri, bool is_ta, char ***args, size_t *args_len)
        return 0;
 }
 
-static void
+__dead static void
 handle_child_thread(char **args, int fds[2][2])
 {
        /* THIS FUNCTION MUST NEVER RETURN!!! */
@@ -280,17 +280,16 @@ create_pipes(int fds[2][2])
        return 0;
 }
 
-static void
+static int
 log_buffer(char const *buffer, ssize_t read, int type, bool log_operation)
 {
 #define PRE_RSYNC "[RSYNC exec]: "
        char *cpy, *cur, *tmp;
 
        cpy = malloc(read + 1);
-       if (cpy == NULL) {
-               pr_enomem();
-               return;
-       }
+       if (cpy == NULL)
+               return pr_enomem();
+
        strncpy(cpy, buffer, read);
        cpy[read] = '\0';
 
@@ -312,6 +311,7 @@ log_buffer(char const *buffer, ssize_t read, int type, bool log_operation)
                cur = tmp + 1;
        }
        free(cpy);
+       return 0;
 #undef PRE_RSYNC
 }
 
@@ -320,6 +320,7 @@ read_pipe(int fd_pipe[2][2], int type, bool log_operation)
 {
        char buffer[4096];
        ssize_t count;
+       int error;
 
        while (1) {
                count = read(fd_pipe[type][0], buffer, sizeof(buffer));
@@ -332,7 +333,9 @@ read_pipe(int fd_pipe[2][2], int type, bool log_operation)
                if (count == 0)
                        break;
 
-               log_buffer(buffer, count, type, log_operation);
+               error = log_buffer(buffer, count, type, log_operation);
+               if (error)
+                       return error;
        }
        close(fd_pipe[type][0]); /* Close read end */
        return 0;
@@ -466,9 +469,6 @@ do_rsync(struct rpki_uri *uri, bool is_ta, bool log_operation)
                        TA_DEBUG_MSG("%d", error);
                        pr_val_debug("Child terminated with error code %d.",
                            error);
-                       if (error == -ENOMEM)
-                               pr_enomem();
-
                        if (!error)
                                goto release_args;
 
index 6c8bbd4a525eebe9cce25e614594db7731a416f4..1ed8532bb2df3e9fcf4bb6b6a35f97c4d03bccf8 100644 (file)
@@ -67,7 +67,7 @@ handle_serial_query_pdu(int fd, struct rtr_request const *request)
                error = send_cache_reset_pdu(fd, version);
                goto end;
        case -ENOMEM: /* Memory allocation failure */
-               pr_enomem();
+               error = pr_enomem();
                goto end;
        case EAGAIN: /* Too many threads */
                /*
index 60569bf15583b251eb76dfa6d9f44f403e7c6001..00943f3507cf64a1dd7516b7bf50a8f70e4af84c 100644 (file)
@@ -227,7 +227,7 @@ read_string(struct pdu_reader *reader, uint32_t string_len, rtr_char **result)
         */
 
        string = malloc(string_len + 1); /* Include NULL chara. */
-       if (!string)
+       if (string == NULL)
                return pr_enomem();
 
        memcpy(string, reader->buffer, string_len);
index 2a962c0e66d1873a6c1819515357e6e426dcf650..564e46c09591cdd5557c43b2f81dd12545839480 100644 (file)
@@ -386,25 +386,17 @@ db_slurm_vrp_is_filtered(struct db_slurm *db, struct vrp const *vrp)
 bool
 db_slurm_bgpsec_is_filtered(struct db_slurm *db, struct router_key const *key)
 {
+       unsigned char ski[RK_SKI_LEN];
        struct slurm_bgpsec slurm_bgpsec;
-       unsigned char *tmp;
-       bool result;
 
-       tmp = malloc(RK_SKI_LEN);
-       if (tmp == NULL) {
-               pr_enomem();
-               return false;
-       }
        slurm_bgpsec.data_flag = SLURM_COM_FLAG_ASN | SLURM_BGPS_FLAG_SKI;
        slurm_bgpsec.asn = key->as;
-       memcpy(tmp, key->ski, RK_SKI_LEN);
-       slurm_bgpsec.ski = tmp;
+       memcpy(ski, key->ski, RK_SKI_LEN);
+       slurm_bgpsec.ski = ski;
        /* Router public key isn't used at filters */
        slurm_bgpsec.router_public_key = NULL;
 
-       result = bgpsec_filtered(db, &slurm_bgpsec);
-       free(tmp);
-       return result;
+       return bgpsec_filtered(db, &slurm_bgpsec);
 }
 
 #define ITERATE_LIST_FUNC(type, object, db_list)                       \
index 392515ba6558de6c8ca47d5eda0d69d60fe3a68e..d640aa49bedd9a14d92aabc85ab710b38dcaa2b0 100644 (file)
--- a/src/uri.c
+++ b/src/uri.c
@@ -168,7 +168,7 @@ ia5str2global(struct rpki_uri *uri, char const *mft, IA5String_t *ia5)
        slash_pos = strrchr(mft, '/');
        if (slash_pos == NULL) {
                joined = malloc(ia5->size + 1);
-               if (!joined)
+               if (joined == NULL)
                        return pr_enomem();
                strncpy(joined, (char *) ia5->buf, ia5->size);
                joined[ia5->size] = '\0';
@@ -178,7 +178,7 @@ ia5str2global(struct rpki_uri *uri, char const *mft, IA5String_t *ia5)
 
        dir_len = (slash_pos + 1) - mft;
        joined = malloc(dir_len + ia5->size + 1);
-       if (!joined)
+       if (joined == NULL)
                return pr_enomem();
 
        strncpy(joined, mft, dir_len);