Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/ingenic: drm_mode_config_create_initial_state() called before private_obj --
commit 7ffae382d87f4623df938c8f5b08713c0f72e24e Author: Maxime Ripard <[email protected]> drm/ingenic: Switch to drm_mode_config_create_initial_state This commit replaces the probe-time call to drm_mode_config_reset() in ingenic_drm_bind() with drm_mode_config_create_initial_state() and checks its return value. > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > index 625ec76cf04d8..63198fd594984 100644 > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > @@ -1354,7 +1354,9 @@ static int ingenic_drm_bind(struct device *dev, bool > has_components) > goto err_drvdata; > } > > - drm_mode_config_reset(drm); > + ret = drm_mode_config_create_initial_state(drm); > + if (ret) > + goto err_drvdata; [Severity: Medium] Should drm_mode_config_create_initial_state(drm) be called after priv->private_obj is initialized with drm_atomic_private_obj_init(), or should drm_atomic_private_obj_init() be moved before this call? In drivers/gpu/drm/drm_mode_config.c, drm_mode_config_create_initial_state() iterates over dev->mode_config.privobj_list to allocate the initial state for registered private objects and propagate any allocation error: drivers/gpu/drm/drm_mode_config.c:drm_mode_config_create_initial_state() { drm_for_each_privobj(privobj, dev) { struct drm_private_state *privobj_state; if (privobj->state) continue; if (!privobj->funcs->atomic_create_state) continue; privobj_state = privobj->funcs->atomic_create_state(privobj); if (IS_ERR(privobj_state)) return PTR_ERR(privobj_state); privobj->state = privobj_state; } ... } In drivers/gpu/drm/ingenic/ingenic-drm-drv.c, ingenic_drm_bind() currently initializes priv->private_obj later in the function without checking its return value: drivers/gpu/drm/ingenic/ingenic-drm-drv.c:ingenic_drm_bind() { ... ret = drm_mode_config_create_initial_state(drm); if (ret) goto err_drvdata; ... drm_atomic_private_obj_init(drm, &priv->private_obj, &ingenic_drm_private_state_funcs); ... } Because priv->private_obj is not yet on drm->mode_config.privobj_list when drm_mode_config_create_initial_state(drm) runs, it is skipped. If ingenic_drm_create_state() fails with -ENOMEM during drm_atomic_private_obj_init() (or once drm_atomic_private_obj_init() stops allocating state directly), priv->private_obj is left on drm->mode_config.privobj_list with priv->private_obj.state set to NULL. When the first atomic commit duplicates the private state (for example via drm_client_setup()), ingenic_drm_duplicate_state() will dereference NULL: drivers/gpu/drm/ingenic/ingenic-drm-drv.c:ingenic_drm_duplicate_state() { struct ingenic_drm_private_state *state = to_ingenic_drm_priv_state(obj->state); state = kmemdup(state, sizeof(*state), GFP_KERNEL); ... } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=20
