Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 9 Dec 2021 17:30:06 -0500 | Subject | Re: Potential Bug in drm/amd/display/dc_link | From | Harry Wentland <> |
| |
On 2021-12-09 03:02, Yizhuo Zhai wrote: > Hi All: > I just found a bug in the cramfs using the static analysis tool, but > not sure if this could happen in reality, could you please advise me > here? Thanks for your attention : ) And please ignore the last one > with HTML format if you did not filter it out. > > In function enable_stream_features(), the variable > "old_downspread.raw" could be uninitialized if core_link_read_dpcd > fails(), however, it is used in the later if statement, and further, > core_link_write_dpcd() may write random value, which is potentially > unsafe. But this function does not return the error code to the up > caller and I got stuck in drafting the patch, could you please advise > me here? >
Thanks for highlighting this.
Unfortunately we frequently ignore DPCD error codes.
In this case I would do a memset as shown below.
> The related code: > static void enable_stream_features(struct pipe_ctx *pipe_ctx) > { > union down_spread_ctrl old_downspread;
memset(&old_downspread, 0, sizeof(old_downspread));
> core_link_read_dpcd(link, DP_DOWNSPREAD_CTRL, > &old_downspread.raw, sizeof(old_downspread); > > //old_downspread.raw used here > if (new_downspread.raw != old_downspread.raw) { > core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL, > &new_downspread.raw, sizeof(new_downspread)); > } > } > enum dc_status core_link_read_dpcd( > struct dc_link *link, > uint32_t address, > uint8_t *data, > uint32_t size) > { > //data could be uninitialized if the helpers fails and log > some error info > if (!dm_helpers_dp_read_dpcd(link->ctx, > link,address, data, size)) > return DC_ERROR_UNEXPECTED; > return DC_OK; > } > > The same issue in function wait_for_training_aux_rd_interval() in > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
I don't see this. Do you mean this one?
> void dp_wait_for_training_aux_rd_interval( > struct dc_link *link, > uint32_t wait_in_micro_secs) > { > #if defined(CONFIG_DRM_AMD_DC_DCN) > if (wait_in_micro_secs > 16000) > msleep(wait_in_micro_secs/1000); > else > udelay(wait_in_micro_secs); > #else > udelay(wait_in_micro_secs); > #endif > > DC_LOG_HW_LINK_TRAINING("%s:\n wait = %d\n", > __func__, > wait_in_micro_secs); > }
Thanks, Harry
>
| ![\](/images/icornerr.gif) |