[Encode] VP9 Encode Add boundary check for log2_tile_columns #1964
Conversation
| col_data = picParam->log2_tile_columns - min_log2_tile_cols; | ||
| if (picParam->log2_tile_columns < min_log2_tile_cols) | ||
| { | ||
| col_data = 0; |
There was a problem hiding this comment.
This is not a bug fix, but rather a protective check for problematic parameters passed down from the APP. However, if the condition "picParam->log2_tile_columns < min_log2_tile_cols" is met, should the driver return directly? After all, if we continue execution, there might be other risks？
There was a problem hiding this comment.
This is boundary check to avoid the crash or long while loop with possible col_data negative value in difference calculation.
We are hitting crash in Android CTS test. where width is 8192.
As per the calculation of min_log2_tile_cols in here it gives 1. But the picParam->log2_tile_columns is passed as 0. which acceptable as per the structure definition.
if value of log2_tile_columns in picParam is 0, Then there should not be any risk in encoding with that configuration.
There was a problem hiding this comment.
- Agreed on adding boundary check here.
- To avoid risks in subsequent processing, I also suggest correcting picParam->log2_tile_columns to min_log2_tile_cols. Since picParam is an internal parameter, it should be safe to modify.
- Please add a warning message here to report this issue.
There was a problem hiding this comment.
@RachelChengRJ
Completely agree with your point 2. I have modified the code to correct the picParam->log2_tile_columns.
Added a note in the code for future reference and Added the DDI_CODEC_ASSERTMESSAGE to get in the logs if this happens, or please suggest if any other log can be used ?
User might pass picParam->log2_tile_columns as 0 for >4K width In this scenario Vp9WriteUncompressHeader may crash in while loop or huze loop due to possible col_data negative value. To avoid this added a boundary check and making picParam->log2_tile_columns same as min_log2_tile_cols. Signed-off-by: sachin kumar <[email protected]>
kumarsac
force-pushed
the
vp9_encode_fix
branch
from
888d2e7 to
89e79cb
Compare
kumarsac
changed the title
2 participants
User might pass picParam->log2_tile_columns as 0 for >4K width
In this scenario Vp9WriteUncompressHeader may crash in while loop
or huze loop due to possible col_data negative value.
To avoid this added a boundary check and making picParam->log2_tile_columns
same as min_log2_tile_cols.