Re: [PATCH v4 2/3] drm/bridge: analogix_dp: Add validation for samsung,lane-count property

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

On Sat May 30, 2026 at 3:33 PM CEST, Luca Ceresoli wrote:
> On Fri, 29 May 2026 12:05:29 +0800, Damon Ding <damon.ding@rock-chips.com> wrote:
>
> Hello Damon,
>
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 8cf6b73bceac..699a7f380c56 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1260,8 +1261,16 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>>  		 */
>>  		of_property_read_u32(dp_node, "samsung,link-rate",
>>  				     &video_info->max_link_rate);
>> -		of_property_read_u32(dp_node, "samsung,lane-count",
>> -				     &video_info->max_lane_count);
>> +		ret = of_property_read_u32(dp_node, "samsung,lane-count",
>> +					   &video_info->max_lane_count);
>> +		if (!ret) {
>> +			if (video_info->max_lane_count == 0 ||
>> +			    video_info->max_lane_count > LANE_COUNT4) {
>
> This sashiko report seems to me valid.

Meh, messed up with 'b4 review' :-/ Apologies

"This sashiko report" [0] was about an enum being signed, so '== 0' could miss
negative numbers coming from bogus DT values higher than 1^31.

>
> But I'n no DP expert, I have no idea whether this ther one is valid.

And this was about "Additionally, does this check inadvertently allow 3,
which is an invalid DisplayPort lane count?"

>> +				dev_err(dp->dev, "samsung,lane-count = %d is out of range\n",
>> +					video_info->max_lane_count);
>> +				return -EINVAL;
>> +			}
>> +		}
>
> As reported by sashiko, 'count == 0' should be 'count <= 0', being an enum.
>
> Additionally I'd avoid the nested if, and I think using dev_err_probe() is
> correct here (we are only called by probe functions), so it all could
> become:
>
>    if (ret || count <= 0 || count > LANE_COUNT0)
>         return dev_err_probe(...);
>
> There are other sashiko reports to patch 3, and at least one seems valid to
> me. Can you either fix them in the next iteration or elaborate on why the
> code is correct there?
>
> Thanks!


[0] https://sashiko.dev/#/patchset/20260529040530.741336-1-damon.ding%40rock-chips.com

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com