Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] Double-free of the firmware object in the error paths of firmware update. - [High] Missing minimum firmware size check causes massive buffer overflow/underflow. - [High] drm_atomic_private_obj_init() leaves the object in privobj_list on error, and drm_bridge_attach() ignores the error, leading to crashes. -- --- Patch [8]: [PATCH v2 08/37] drm/bridge: lontium-lt8713sx: Add atomic bridge state handlers --- commit cc389f313911703ff4573192b0ab28ddca13b386 Author: Maxime Ripard <[email protected]> drm/bridge: lontium-lt8713sx: Add atomic bridge state handlers The lontium-lt8713sx bridge doesn't implement any enable or disable callbacks, but is still missing the atomic state handlers. In order to remove the legacy bridge callback support from the DRM bridge core, add them. > diff --git a/drivers/gpu/drm/bridge/lontium-lt8713sx.c > b/drivers/gpu/drm/bridge/lontium-lt8713sx.c > index cee485adf5e56..4a6420b876da6 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt8713sx.c > +++ b/drivers/gpu/drm/bridge/lontium-lt8713sx.c [ ... ] > @@ -508,6 +509,9 @@ static const struct attribute_group > *lt8713sx_attr_groups[] = { > }; > > static const struct drm_bridge_funcs lt8713sx_bridge_funcs = { > + .atomic_create_state = drm_atomic_helper_bridge_create_state, > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > .attach = lt8713sx_bridge_attach, > }; [Severity: High] This isn't a bug introduced by this patch, but by adding .atomic_create_state you are enabling the atomic path in drm_bridge_attach() which will now call drm_atomic_private_obj_init(). If drm_atomic_private_obj_init() fails, it returns an error but leaves the object in dev->mode_config.privobj_list with a NULL obj->state: drm_atomic.c:drm_atomic_private_obj_init() list_add_tail(&obj->head, &dev->mode_config.privobj_list); state = obj->funcs->atomic_create_state(obj); if (IS_ERR(state)) return PTR_ERR(state); obj->state = state; Then drm_bridge_attach() ignores this return value: drm_bridge.c:drm_bridge_attach() if (drm_bridge_is_atomic(bridge)) drm_atomic_private_obj_init(bridge->dev, &bridge->base, &drm_bridge_priv_state_funcs); return 0; If a state dump is triggered later, drm_state_dump() iterates over privobj_list and calls drm_atomic_private_obj_print_state(p, obj->state). Since obj->state is NULL, this will cause a NULL pointer dereference. Should the return value of drm_atomic_private_obj_init() be checked in drm_bridge_attach(), and should drm_atomic_private_obj_init() remove the object from the list on failure? [Severity: High] This is a pre-existing issue, but I noticed a potential double-free in the firmware update error paths of this driver. In lt8713sx_prepare_firmware_data(): release_firmware(lt8713sx->fw); return -EINVAL; This frees the firmware object but leaves lt8713sx->fw pointing to the freed memory. Then the caller lt8713sx_firmware_update() handles the error: error: if (lt8713sx->fw) { release_firmware(lt8713sx->fw); lt8713sx->fw = NULL; } Could this result in a double-free of the firmware object? [Severity: High] This is a pre-existing issue, but I also noticed a potential buffer overflow in lt8713sx_prepare_firmware_data(): memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, SZ_64K - 1); ... memcpy(lt8713sx->fw_buffer + SZ_64K, lt8713sx->fw->data + SZ_64K, lt8713sx->fw->size - SZ_64K); If a user provides a firmware file smaller than 64KB, would the first memcpy read out of bounds, and would lt8713sx->fw->size - SZ_64K underflow resulting in a massive out-of-bounds copy? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
