verbs: a message or an answer there is no memory for is answered -901 (#174)

.gitignore

@@ -19,6 +19,7 @@ tests/rt_interpose.so
 tests/gesture-alloc-test
 tests/lv2-alloc-test
 tests/lv2-alloc-lv2.o
+tests/verbs-alloc-test
 tests/transforms-test
 /build/
 fuzz/findings/

Makefile

@@ -10,6 +10,7 @@ PIN_TEST = tests/pin-test
 RELAY_TEST = tests/relay-test
 GESTURE_ALLOC_TEST = tests/gesture-alloc-test
 LV2_ALLOC_TEST = tests/lv2-alloc-test
+VERBS_ALLOC_TEST = tests/verbs-alloc-test
 TRANSFORMS_TEST = tests/transforms-test
 PROBE = plugin-hostd-probe
 
@@ -249,7 +250,7 @@ fuzz-corpus-check:
 
 # clean rule
 clean:
-	@rm -f src/*.o $(PROG) $(PROBE) tests/probe-child tests/lv2-probe-child tests/rt_interpose.so $(FAKE) $(PIN_TEST) $(RELAY_TEST) tests/relay-test-relay.o $(FORMAT_TEST) $(GESTURE_ALLOC_TEST) $(LV2_ALLOC_TEST) tests/lv2-alloc-lv2.o $(TRANSFORMS_TEST) $(GEN) tests/stress.clap tests/jack_levels tests/host_scenarios $(LV2_FIXTURE)
+	@rm -f src/*.o $(PROG) $(PROBE) tests/probe-child tests/lv2-probe-child tests/rt_interpose.so $(FAKE) $(PIN_TEST) $(RELAY_TEST) tests/relay-test-relay.o $(FORMAT_TEST) $(GESTURE_ALLOC_TEST) $(LV2_ALLOC_TEST) tests/lv2-alloc-lv2.o $(VERBS_ALLOC_TEST) $(TRANSFORMS_TEST) $(GEN) tests/stress.clap tests/jack_levels tests/host_scenarios $(LV2_FIXTURE)
 	@rm -rf build
 
 # the daemon against workers that are not plugin hosts at all (tests/fake-host speaks the same protocol and can be
@@ -287,8 +288,8 @@ test-socket: $(PROG)
 test-format: $(FORMAT_TEST) $(FAKE)
 	PLUGIN_HOSTD_FORMAT_TEST_DIR=$${TMPDIR:-$${XDG_RUNTIME_DIR:-.}} ./$(FORMAT_TEST) ./$(FAKE)
 
-# the daemon's parts without its main and its verbs: the test calls the supervisor as the verbs do
-FORMAT_OBJ = src/conf.o src/proc.o src/supervisor.o src/pins.o src/relay.o src/formats.o src/lv2.o
+# the daemon's parts without its main and its verbs: format_test calls the supervisor as the verbs do
+FORMAT_OBJ = $(filter-out src/main.o src/verbs.o,$(OBJ))
 
 $(FORMAT_TEST): tests/format_test.c $(FORMAT_OBJ) $(PROTOCOL_LIB)
 	$(CC) $(INCS) -Isrc $(CFLAGS) -Werror -o $@ tests/format_test.c $(FORMAT_OBJ) $(PROTOCOL_LIBS) -pthread -lm
@@ -315,11 +316,12 @@ test-relay: $(RELAY_TEST)
 $(RELAY_TEST): tests/relay_test.c src/relay.c src/relay.h include/plugin-hostd/protocol.h $(RENDERED)
 	$(CC) -Iinclude $(CFLAGS) -Werror -Drealloc=relay_test_realloc -c -o tests/relay-test-relay.o src/relay.c
 	$(CC) -Isrc $(CFLAGS) -Werror -o $@ tests/relay_test.c tests/relay-test-relay.o -pthread
-# the relay's gesture table and lv2.c's refusal and pick tables when memory runs out: each is built with its realloc
-# calls renamed, so the test picks the one that fails
-test-alloc: $(GESTURE_ALLOC_TEST) $(LV2_ALLOC_TEST)
+# the relay's gesture table, lv2.c's refusal and pick tables, and the verbs' copies and answer when memory runs out:
+# each is built with its allocation calls renamed, so the test picks the one that fails
+test-alloc: $(GESTURE_ALLOC_TEST) $(LV2_ALLOC_TEST) $(VERBS_ALLOC_TEST)
 	./$(GESTURE_ALLOC_TEST)
 	./$(LV2_ALLOC_TEST)
+	./$(VERBS_ALLOC_TEST)
 
 $(GESTURE_ALLOC_TEST): tests/gesture_alloc_test.c src/relay.c src/relay.h include/plugin-hostd/protocol.h $(RENDERED)
 	$(CC) -Iinclude -Isrc $(CFLAGS) -Werror -Drealloc=alloc_test_realloc -o $@ tests/gesture_alloc_test.c -pthread
@@ -328,6 +330,10 @@ $(LV2_ALLOC_TEST): tests/lv2_alloc_test.c src/lv2.c src/formats.h
 	$(CC) $(INCS) -Isrc $(CFLAGS) -Werror -Drealloc=alloc_test_realloc -c -o tests/lv2-alloc-lv2.o src/lv2.c
 	$(CC) $(INCS) -Isrc $(CFLAGS) -Werror -o $@ tests/lv2_alloc_test.c tests/lv2-alloc-lv2.o
 
+$(VERBS_ALLOC_TEST): tests/verbs_alloc_test.c src/verbs.c src/verbs.h $(FORMAT_OBJ) $(PROTOCOL_LIB)
+	$(CC) $(INCS) -Isrc $(CFLAGS) -Werror -Dstrndup=alloc_test_strndup -Dstrdup=alloc_test_strdup \
+	  -Dfree=alloc_test_free -o $@ tests/verbs_alloc_test.c $(FORMAT_OBJ) $(PROTOCOL_LIBS) -pthread -lm
+
 tests/stress.clap: tests/stress_plugin.c
 	$(CC) $(CLAP_CFLAGS) $(CFLAGS) -Werror -shared -o $@ $<
 

data/verbs.json

@@ -146,6 +146,9 @@
         "format": "worker_list",
         "reply": "resp <n> <worker>:<pid>:<format>:<state>:<place>:<i>,<i>,...",
         "codes": [
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -163,6 +166,9 @@
           {
             "ref": "HOST_REPLY_CODES.instanceNonExists"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -180,6 +186,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.noSuchWorker"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -197,6 +206,9 @@
           {
             "ref": "HOST_REPLY_CODES.instanceNonExists"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -214,6 +226,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.placementInvalid"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -231,6 +246,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.placementInvalid"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -245,6 +263,9 @@
         "format": "worker_env %s %s %s",
         "reply": "resp 0",
         "codes": [
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -262,6 +283,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.pinRefused"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -276,6 +300,9 @@
         "format": "pin_clear %s",
         "reply": "resp 0",
         "codes": [
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -299,6 +326,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.pinRefused"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -323,6 +353,9 @@
         "format": "lv2_worker_set %s %s",
         "reply": "resp 0",
         "codes": [
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -352,6 +385,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.placementInvalid"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -371,6 +407,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.noSuchChain"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -388,6 +427,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.noSuchChain"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -414,6 +456,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.descriptorScanFailed"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -440,6 +485,9 @@
           {
             "ref": "PLUGIN_HOSTD_REPLY_CODES.descriptorScanFailed"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -460,6 +508,9 @@
           {
             "ref": "HOST_REPLY_CODES.instanceNonExists"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }
@@ -482,6 +533,9 @@
           {
             "ref": "HOST_REPLY_CODES.instanceNonExists"
           },
+          {
+            "ref": "HOST_REPLY_CODES.memoryAllocation"
+          },
           {
             "ref": "HOST_REPLY_CODES.invalidOperation"
           }

fuzz/README.md

@@ -76,7 +76,7 @@ seed.
 
 | Harness | Lines | Covered | Lines no seed reaches |
 | --- | ---: | ---: | --- |
-| command | 153 | 100.00% | |
+| command | 178 | 92.13% | the paths taken when the message's copies or an answer get no memory (`tests/verbs_alloc_test.c` drives them) |
 | reply | 210 | 90.00% | the paths taken when `realloc` fails (`tests/relay_test.c` and `tests/gesture_alloc_test.c` drive them), the relay thread's join and the listening socket in `relay_finish`, and the `fd < 0` guards of `relay_add` and `relay_remove` |
 
 The records

fuzz/coverage-floor.txt

@@ -1,4 +1,4 @@
 # Covered lines each harness's seed corpus reaches (make fuzz-coverage, fuzz/coverage.sh).
 # A harness below its floor turns fuzz-coverage red. harness covered-lines
-command 155
+command 164
 reply 189

fuzz/fuzz_command.c

@@ -203,19 +203,19 @@ __attribute__((noinline)) static void check_reply_answer(const struct words *w)
     if (g_rec.answer)
     {
         /* a call answered: the reply is its answer, as it answered it
-         * mutant: src/verbs.c:120 `text, strlen(text) + 1` -> `text + 1, strlen(text)`
+         * mutant: src/verbs.c:129 `text, strlen(text) + 1` -> `text + 1, strlen(text)`
          * input: add-default
          * mutant: src/verbs.c:90 `return rc;` -> `return SUCCESS;`
          * input: plugin-hostd-pin_set-refused
-         * mutant: src/verbs.c:220 `return reply_text;` -> `return sup_resp(SUCCESS);`
+         * mutant: src/verbs.c:229 `return reply_text;` -> `return sup_resp(SUCCESS);`
          * input: mod-host-set_bpm-refused */
         if (strcmp(g_rec.reply, g_rec.answer))
             abort();
         return;
     }
     /* no call answered: the reply is one the daemon makes itself, a refusal or the reply of quit, which check_reply
      * holds to "resp <code>" of a declared code and to "resp 0"
-     * mutant: src/verbs.c:135 `return sup_resp(ERR_INVALID_OPERATION);` -> `return sup_resp(SUCCESS);`
+     * mutant: src/verbs.c:144 `return sup_resp(ERR_INVALID_OPERATION);` -> `return sup_resp(SUCCESS);`
      * input: add-too-few */
     if (sup_reply_code(g_rec.reply) >= 0 && !(w->n == 1 && verb_is(w->w[0], QUIT)))
         abort();
@@ -229,12 +229,12 @@ __attribute__((noinline)) static void check_reply(const struct words *w)
     int code;
 
     /* one reply, and nothing sent empty
-     * mutant: src/verbs.c:238 `reply(msg->sender_id, handle(line, tok, ntok));` -> `free(handle(line, tok, ntok));`
+     * mutant: src/verbs.c:259 `reply(msg->sender_id, handle(line, tok, ntok));` -> `free(handle(line, tok, ntok));`
      * input: add-default */
     if (g_rec.nreplies != 1 || g_rec.reply_size == 0)
         abort();
     /* the reply is a string that ends where the message the socket sends ends
-     * mutant: src/verbs.c:120 `strlen(text) + 1` -> `strlen(text)`
+     * mutant: src/verbs.c:129 `strlen(text) + 1` -> `strlen(text)`
      * input: add-default */
     if (g_rec.reply[g_rec.reply_size - 1] != '\0' || memchr(g_rec.reply, '\0', g_rec.reply_size - 1))
         abort();
@@ -252,7 +252,7 @@ __attribute__((noinline)) static void check_reply(const struct words *w)
     }
     shape = w->n ? rec_shape_of(w->w[0], strlen(w->w[0])) : REC_MOD_HOST_SHAPE;
     /* anything else has the shape its verb declares, or mod-host's
-     * mutant: src/verbs.c:209 `sup_resp(SUCCESS)` -> `sup_resp(1)`
+     * mutant: src/verbs.c:218 `sup_resp(SUCCESS)` -> `sup_resp(1)`
      * input: mod-host-quit */
     if (!syntax_reply_matches(shape, g_rec.reply) && !syntax_reply_matches(REC_MOD_HOST_SHAPE, g_rec.reply))
         abort();
@@ -286,9 +286,9 @@ __attribute__((noinline)) static void check_arity(const struct words *w)
         else
             syntax_arity(g_parsed[n].syntax, &min, &max);
         /* acted on, by its own call or by a broadcast, with an argument count the declaration allows
-         * mutant: src/verbs.c:175 `ntok == 3` -> `ntok >= 3`
+         * mutant: src/verbs.c:184 `ntok == 3` -> `ntok >= 3`
          * input: plugin-hostd-pool_config-extra
-         * mutant: src/verbs.c:152 `verb_is(verb, EFFECT_REMOVE)` -> `verb_is(verb, EFFECT_REMOVE) && ntok == 2`
+         * mutant: src/verbs.c:161 `verb_is(verb, EFFECT_REMOVE)` -> `verb_is(verb, EFFECT_REMOVE) && ntok == 2`
          * input: mod-host-remove-extra */
         if (read - 1 < min || read - 1 > max)
             abort();
@@ -346,7 +346,7 @@ __attribute__((noinline)) static void check_words_add(const struct rec_call *cal
     const char *client = read > next ? w[next] : "-";
 
     /* the placement and the client, each where the message has it
-     * mutant: src/verbs.c:142 `placement, client` -> `client, placement`
+     * mutant: src/verbs.c:151 `placement, client` -> `client, placement`
      * input: add-pool-p-fuzz_client */
     if (call->nargs != 4 || strcmp(call->args[2], placement) || strcmp(call->args[3], client))
         abort();
@@ -429,7 +429,7 @@ __attribute__((noinline)) static void check_words(const struct words *w)
             }
             item += strcspn(item, " ");
             /* the argument is the word the declaration puts there, as the controller wrote it
-             * mutant: src/verbs.c:142 `atoi(tok[2])` -> `atoi(tok[1])`
+             * mutant: src/verbs.c:151 `atoi(tok[2])` -> `atoi(tok[1])`
              * input: add-instance-past-the-table */
             if (want && (call->args[a] == NULL || strcmp(call->args[a], want)))
                 abort();
@@ -450,7 +450,7 @@ __attribute__((noinline)) static void check_refusal(uint64_t before)
     if (g_rec.reply == NULL || sup_reply_code(g_rec.reply) >= 0)
         return;
     /* nothing changed: no accepted call, no follow-up, no quit
-     * mutant: src/verbs.c:144 `reply_text && sup_reply_code(reply_text) >= 0` -> `reply_text`
+     * mutant: src/verbs.c:153 `reply_text && sup_reply_code(reply_text) >= 0` -> `reply_text`
      * input: add-instance-past-the-table */
     if (g_rec.state != before || g_quitting)
         abort();
@@ -485,17 +485,17 @@ __attribute__((noinline)) static void check_round_trip(const struct words *w, ui
     free(line);
 
     /* the same calls with the same arguments
-     * mutant: src/verbs.c:232 `strtok_r(copy, " \t", &save)` -> `strtok_r(copy, " ", &save)`
+     * mutant: src/verbs.c:253 `strtok_r(copy, " \t", &save)` -> `strtok_r(copy, " ", &save)`
      * input: mod-host-add-tabs */
     if (!rec_calls_equal(first, nfirst, g_rec.calls, g_rec.ncalls))
         abort();
     /* the same reply
-     * mutant: src/verbs.c:206 `ntok != 1` -> `ntok != 1 || line[0] != ' '`
+     * mutant: src/verbs.c:215 `ntok != 1` -> `ntok != 1 || line[0] != ' '`
      * input: quit-leading-space */
     if (g_rec.reply == NULL || strcmp(g_rec.reply, reply_first))
         abort();
     /* the same state, the same host settings, and quitting the same
-     * mutant: src/verbs.c:208 `g_quitting = 1;` -> `g_quitting = line[0] == ' ';`
+     * mutant: src/verbs.c:217 `g_quitting = 1;` -> `g_quitting = line[0] == ' ';`
      * input: quit-leading-space */
     if (g_quitting != quit_first || g_rec.state != state_first || g_rec.host_settings != host_first)
         abort();

fuzz/sabotage/command-reply.patch

@@ -2,12 +2,12 @@ Breaks the check of fuzz/fuzz_command.c:240 (the reply is a string that ends whe
 Caught by the seed input "add-default" of fuzz_command.
 
 diff --git a/src/verbs.c b/src/verbs.c
-index 78daab7..6b8af56 100644
+index aebc6c6..4859ca8 100644
 --- a/src/verbs.c
 +++ b/src/verbs.c
-@@ -118,5 +118,5 @@ static char *chain_words(char **tok, int ntok)
- static void reply(int fd, char *text)
- {
+@@ -127,5 +127,5 @@ static void reply(int fd, char *text)
+         return;
+     }
 -    socket_send(fd, text, strlen(text) + 1);
 +    socket_send(fd, text, strlen(text));
      free(text);

fuzz/sabotage/command-round-trip.patch

@@ -2,12 +2,12 @@ Breaks the check of fuzz/fuzz_command.c:490 (a command's words joined by one spa
 Caught by the seed input "mod-host-add-tabs" of fuzz_command.
 
 diff --git a/src/verbs.c b/src/verbs.c
-index 78daab7..bf45b47 100644
+index aebc6c6..a715349 100644
 --- a/src/verbs.c
 +++ b/src/verbs.c
-@@ -222,5 +222,5 @@ void verbs_receive(msg_t *msg)
-         *--end = '\0';
-     copy = strdup(line);
+@@ -251,5 +251,5 @@ void verbs_receive(msg_t *msg)
+         return;
+     }
 -    for (t = strtok_r(copy, " \t", &save); t && ntok < MAX_TOKENS; t = strtok_r(NULL, " \t", &save))
 +    for (t = strtok_r(copy, " ", &save); t && ntok < MAX_TOKENS; t = strtok_r(NULL, " \t", &save))
          tok[ntok++] = t;