Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] rockchip/dp: unchecked NULL pointer dereference on missing GRF property --
commit 9a6e6f45c854c22f24d403cf8bdab7a0504199b6 Author: Damon Ding <[email protected]> drm/rockchip: analogix_dp: Add support for RK3568 The RK3568 eDP controller does not require any GRF configurations, so make the "rockchip,grf" property optional. Also enable the optional avdd-0v9 and avdd-1v8 supplies which power the eDP PHY. > diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > @@ -320,8 +320,9 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device > *dp) > struct device *dev = dp->dev; > struct device_node *np = dev->of_node; > struct clk *clk; > + int ret; > > - dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); > + dp->grf = syscon_regmap_lookup_by_phandle_optional(np, "rockchip,grf"); > if (IS_ERR(dp->grf)) > return dev_err_probe(dev, PTR_ERR(dp->grf), > "failed to get rockchip,grf property\n"); [Severity: High] Since this changes the lookup to syscon_regmap_lookup_by_phandle_optional(), dp->grf will be NULL when the property is absent, and the IS_ERR() check will pass. Can this cause a NULL pointer dereference for legacy SoCs (like RK3288, RK3399, or RK3588) if the device tree is missing the property? For those older SoCs, the chip_data specifies valid GRF fields. During display initialization, the driver calls rockchip_grf_field_write(): drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_grf_field_write() { if (!field->valid) return 0; mask = GENMASK(field->msb, field->lsb); val <<= field->lsb; return rockchip_grf_write(grf, field->reg, mask, val); } This forwards the potentially NULL grf pointer to rockchip_grf_write() and eventually to regmap_write(), which dereferences it: drivers/base/regmap/regmap.c:regmap_write() { ... if (!IS_ALIGNED(reg, map->reg_stride)) return -EINVAL; ... } Should probe validate that dp->grf != NULL if the chip type requires it, rather than allowing probe to succeed and causing a kernel oops later? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
