Skip to content

Clarify demarshalling code

Simon McVittie requested to merge smcv/dbus:clarify-marshalling into master
  • dbus-marshal-validate: Add an extra assertion

    We already checked that claimed_len <= (end - p), therefore we can assume that claimed_len + p <= end. Make this a bit more obvious.

  • dbus-marshal-recursive: Correct an incorrect comment

    array_reader_check_finished() no longer returns a type, only a boolean, so this comment isn't accurate any more.

  • dbus-marshal-validate: Add more comments indicating what is going on

  • dbus-marshal-basic: Clarify what is going on when we skip an item

  • dbus-marshal-basic: Assert that we are in-bounds after skipping items

    We recommend disabling assertions in production builds of dbus, so it is "cheap" to add them even in relatively fast-path locations.

  • dbus-marshal-basic: Fix an incorrect comment

    We have 16-bit types with 2-byte alignment, but this comment claimed we only have 1-, 4- or 8-byte alignment. The actual implementation is fine, and correctly reports 2-byte alignment for the 16-bit types.


Some corrections and clarifications that helped me to fix #413 (closed) (CVE-2022-42011); I didn't push these at the time, to keep the diffstat for the security update manageable. This code is dense and security-sensitive, and a lot of it hasn't been touched for many years, so anything that can help the current maintainers to understand it seems good to have!

No functional changes intended, other than some extra assertions.

/cc @thiago @evverx

Merge request reports