Skip to content

drm/vc4: hdmi: Don't write past the end of a packet RAM slot - #7579

Open
popcornmix wants to merge 1 commit into
raspberrypi:rpi-6.18.yfrom
popcornmix:vc4-infoframe-overrun
Open

drm/vc4: hdmi: Don't write past the end of a packet RAM slot#7579
popcornmix wants to merge 1 commit into
raspberrypi:rpi-6.18.yfrom
popcornmix:vc4-infoframe-overrun

Conversation

@popcornmix

Copy link
Copy Markdown
Collaborator

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.

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>
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