Skip to content

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

Open
sjawhar wants to merge 1 commit into
FreeMixer:mainfrom
sjawhar:verbs-alloc-901
Open

sjawhar wants to merge 1 commit into
FreeMixer:mainfrom
sjawhar:verbs-alloc-901

Conversation

@sjawhar

@sjawhar sjawhar commented Oct 26, 2026 •

Copy link
Copy Markdown

What: verbs_receive (src/verbs.c) used its strndup and strdup copies of the message without a check. reply() used each handler's answer without a check too. When an allocation was refused, the daemon read address 0 and died of SIGSEGV. Now:

  • a message with no memory for either copy is not acted on. The daemon answers resp -901 (mod-host's ERR_MEMORY_ALLOCATION). data/verbs.json declares it on the daemon's own verbs. Shared verbs that plugin-hostd answers declare -901 omx-clap-core#363 declares it on the shared ones.
  • an answer there is no memory for is sent as resp -901 too, written on the stack. The handler has run by then. So a verb that acted (quit, remove, chain, or another that answers through sup_resp) can still be answered -901. This breaks check_refusal's rule (fuzz/fuzz_command.c) that a refusal changes nothing, but no fuzz seed reaches this path. The controller cannot tell whether the verb ran.

How tested:

  • tests/verbs_alloc_test.c (make test-alloc) includes src/verbs.c with strndup, strdup and free renamed, as tests/gesture_alloc_test.c does, to pick the failing copy and see each copy freed.
  • Before this change gcc does not build the test (-Werror=nonnull at reply's strlen(text)). Under clang UBSan it stops at the strlen(line) of the refused copy. After this change, its nine checks pass.

Sabotage: four alloc arms in tests/sabotage.py, one per guard and one for the free(line) after the strdup fails.

With #173 applied, ci.yml's test job and fuzz.yml pass in a fedora:44 container. Without it, the checks that #173 fixes fail now and then, here and at base.

Refs #174

…FreeMixer#174)

verbs_receive used its strndup and strdup copies of the message unchecked. reply() used each handler's answer
unchecked too. When an allocation was refused, the daemon read address 0 and died on that message. A message with
no memory for either copy is now answered "resp -901", mod-host's ERR_MEMORY_ALLOCATION, and is not acted on. An
answer there is no memory for is sent as "resp -901" too, written on the stack. The handler has run by then. So a
verb that already acted (quit, remove, chain, or another that answers through sup_resp) can still be answered -901.
That breaks the rule that a refusal changes nothing, which check_refusal in fuzz/fuzz_command.c checks. No fuzz
seed reaches this path. The controller cannot tell whether the verb ran. data/verbs.json declares -901 on the
daemon's own verbs. The rendered header follows. A pull request to omx-clap-core declares -901 on the shared ones.

tests/verbs_alloc_test.c includes src/verbs.c with strndup, strdup and free renamed, so it picks the copy that
fails and sees each copy freed (make test-alloc). It links the rest of the daemon through FORMAT_OBJ, now derived
from OBJ: its own list lacked descriptors.o, describe.o, transforms.o and json.o. Each guard, and the free of the
line after the strdup fails, has a sabotage arm. The line numbers in fuzz/fuzz_command.c, command-reply.patch and
command-round-trip.patch follow the moved lines. The command rows of fuzz/README.md and fuzz/coverage-floor.txt
count the new lines, which no seed reaches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant