amdv: out-of-bounds and use-after-free hardening#30
Open
allolive wants to merge 16 commits into
Open
Conversation
amdv_clear_buf cleared the vframe slots but never put the enhancement-layer frame in slot [1], unlike the other paths that clear these slots. Collect the EL frames under the lock and put them after unlocking, since the put can sleep. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
The setting struct is reused each frame and the reset loop left in_md/in_comp pointing at the previous frame's buffers. NULL them and clear el_halfsize_flag so a skipped input carries nothing stale. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
tv_hw5_setting/invalid_hw5_setting/last_tv_hw5_setting are vmalloc'd and only partly filled before invalid_hw5_setting is used as a reset setting, leaving its pointer fields uninitialised. Use vzalloc. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
The md_buf clears used dv_inst[0] instead of dv_inst[i]; comp_buf on the next lines uses [i]. Use dv_inst[i]. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
unregister_dv_functions freed pq_config_dvp_fake but NULLed pq_config_fake, leaving the freed pointer set. NULL the pointer that was actually freed. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
The ATSC SEI parse copied the payload from the aux buffer without checking it fits. Stop parsing if the payload would run past the end of aux_buf. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
The sink VSVDB is compared and copied into the 32-byte vsvdb_tbl from the 27-byte rawdata using dv_info->length (a u8, so length+1 up to 256) with no bound, so a malformed EDID length overflows the table and over-reads rawdata. Clamp it to the source size. The same copy exists in both amdv_parse_metadata_v1 and amdv_parse_metadata_v2_stb; clamp both. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
register_dv_functions sets p_funcs_stb before some allocations that can fail and left it set when they do, unlike the TV branches which clear their pointer. Clear p_funcs_stb (and free ko_info) on those failure paths. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
The put was done while holding amdv_lock, and it can sleep. Collect the frames under the lock and put them after unlocking. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
vsem_check used a bitstream-supplied length without bounds for the last packet copy and the trailing CRC read. Reject a too-small length and clamp the copy and the length to the buffer. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
amdolby_vision_debug_store used the parsed tokens and buf_orig without checking kstrdup or that a command was given. Bail (freeing buf_orig) on those, and fill absent tokens with an empty string so the per-command parsing does not read a NULL token. The shared parser is left untouched. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
The AV1 RPU parse read up to rpu_size bytes from the aux buffer without checking they fit. Stop if the read would run past the end of aux_buf. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
parse_sei_and_meta_ext_hw5 checked the block size against the whole aux buffer instead of the bytes left after the cursor. Use the same check as the other parsers. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
The handler used the parsed tokens without a NULL check and never freed buf_orig on the main path. Guard kstrdup/the tokens, fill absent tokens with an empty string, and free buf_orig before returning. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
parse_param filled the caller's parm[] with no limit, but both callers pass an eight-entry array. Stop at eight and fill the rest with an empty string so the callers do not read past the array or use a NULL token. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
amdv_vf_put walked and cleared the vframe slots without the amdv_lock that amdv_clear_buf and the unregister path hold. Take amdv_lock, clear the slots under it, and put the EL frames after unlocking since the put can sleep. Signed-off-by: Olivier Allauzen <olivier.allauzen@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
1 participant
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.
A batch of small safety fixes for the amdolby_vision driver. I went through the metadata, EDID and sysfs parsing paths plus the video-frame handling and cleaned up the spots that could read or write past a buffer, free something twice, or use a pointer after it was gone. None of these change how the driver behaves.