drm/vc4: hdmi: Don't write past the end of a packet RAM slot - #7579
Open
popcornmix wants to merge 1 commit into
Open
drm/vc4: hdmi: Don't write past the end of a packet RAM slot#7579popcornmix wants to merge 1 commit into
popcornmix wants to merge 1 commit into
Conversation
Each packet RAM slot is VC4_HDMI_PACKET_STRIDE (36) bytes: nine 32-bit words holding a 3-byte packet header followed by four 7-byte subpackets. The even words carry three bytes and the odd words four, so a slot can only ever hold 31 bytes of infoframe. vc4_hdmi_write_infoframe() instead loops over the infoframe seven bytes at a time and emits two words per iteration, consuming ceil(len / 7) * 8 bytes of RAM. For any infoframe longer than 28 bytes that is more than the slot holds, and the trailing write lands on the first word of the next slot. The subsequent loop that clears the remainder of the slot is skipped as well, since packet_reg has already passed packet_reg_next. Today the only infoframe large enough to trigger this is the 30-byte Dynamic Range and Mastering infoframe, which occupies slot 7 and spills into slot 8, so nothing observable happens. A full-length (31-byte) vendor infoframe would occupy slot 1 and zero the header word of slot 2, corrupting the AVI infoframe written just before it. Stop the loop once the slot is full, and reject infoframes that cannot fit rather than silently truncating them. The limit matches the largest infoframe hdmi_infoframe_pack() can produce, HDMI_INFOFRAME_HEADER_SIZE + HDMI_MAX_INFOFRAME_SIZE, so no valid caller is affected. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each packet RAM slot is VC4_HDMI_PACKET_STRIDE (36) bytes: nine 32-bit words holding a 3-byte packet header followed by four 7-byte subpackets. The even words carry three bytes and the odd words four, so a slot can only ever hold 31 bytes of infoframe.
vc4_hdmi_write_infoframe() instead loops over the infoframe seven bytes at a time and emits two words per iteration, consuming ceil(len / 7) * 8 bytes of RAM. For any infoframe longer than 28 bytes that is more than the slot holds, and the trailing write lands on the first word of the next slot. The subsequent loop that clears the remainder of the slot is skipped as well, since packet_reg has already passed packet_reg_next.
Today the only infoframe large enough to trigger this is the 30-byte Dynamic Range and Mastering infoframe, which occupies slot 7 and spills into slot 8, so nothing observable happens. A full-length (31-byte) vendor infoframe would occupy slot 1 and zero the header word of slot 2, corrupting the AVI infoframe written just before it.
Stop the loop once the slot is full, and reject infoframes that cannot fit rather than silently truncating them. The limit matches the largest infoframe hdmi_infoframe_pack() can produce,
HDMI_INFOFRAME_HEADER_SIZE + HDMI_MAX_INFOFRAME_SIZE, so no valid caller is affected.