| Message ID | 20230918150043.403250-1-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: panel: Fix device link for DRM_BRIDGE_ATTACH_NO_CONNECTOR | expand |
Hi Angelo, Thank you for the patch. On Monday, September 18, 2023 11:01 PM, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > When external bridges are attached with > DRM_BRIDGE_ATTACH_NO_CONNECTOR, > the panel bridge may also get the same flag, but in the .attach() > callback for the panel bridge a device link is added only when this > flag is not present; To make things worse, the .detach() callback > tries to delete the device link unconditionally and without checking > if it was created in the first place, crashing the kernel with a NULL > pointer kernel panic upon calling panel_bridge_detach(). > > Fix that by moving the device_link_add() call before checking if the > DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is present. > > Fixes: 199cf07ebd2b ("drm/bridge: panel: Add a device link between drm > device and panel device") > Signed-off-by: AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> > --- > drivers/gpu/drm/bridge/panel.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/panel.c > b/drivers/gpu/drm/bridge/panel.c > index e00d2e94c751..8c507dfd589e 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -67,14 +67,6 @@ static int panel_bridge_attach(struct drm_bridge > *bridge, > struct drm_device *drm_dev = bridge->dev; > int ret; > > - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > - return 0; > - > - if (!bridge->encoder) { > - DRM_ERROR("Missing encoder\n"); > - return -ENODEV; > - } > - > panel_bridge->link = device_link_add(drm_dev->dev, panel->dev, > DL_FLAG_STATELESS); > if (!panel_bridge->link) { > @@ -83,6 +75,14 @@ static int panel_bridge_attach(struct drm_bridge > *bridge, > return -EINVAL; > } > > + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > + return 0; > + > + if (!bridge->encoder) { > + DRM_ERROR("Missing encoder\n"); Shouldn't the device link be deleted in case of error? Regards, Liu Ying > + return -ENODEV; > + } > + > drm_connector_helper_add(connector, > &panel_bridge_connector_helper_funcs); > > -- > 2.42.0
Il 19/09/23 03:45, Ying Liu ha scritto: > Hi Angelo, > > Thank you for the patch. > > On Monday, September 18, 2023 11:01 PM, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: >> When external bridges are attached with >> DRM_BRIDGE_ATTACH_NO_CONNECTOR, >> the panel bridge may also get the same flag, but in the .attach() >> callback for the panel bridge a device link is added only when this >> flag is not present; To make things worse, the .detach() callback >> tries to delete the device link unconditionally and without checking >> if it was created in the first place, crashing the kernel with a NULL >> pointer kernel panic upon calling panel_bridge_detach(). >> >> Fix that by moving the device_link_add() call before checking if the >> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is present. >> >> Fixes: 199cf07ebd2b ("drm/bridge: panel: Add a device link between drm >> device and panel device") >> Signed-off-by: AngeloGioacchino Del Regno >> <angelogioacchino.delregno@collabora.com> >> --- >> drivers/gpu/drm/bridge/panel.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/panel.c >> b/drivers/gpu/drm/bridge/panel.c >> index e00d2e94c751..8c507dfd589e 100644 >> --- a/drivers/gpu/drm/bridge/panel.c >> +++ b/drivers/gpu/drm/bridge/panel.c >> @@ -67,14 +67,6 @@ static int panel_bridge_attach(struct drm_bridge >> *bridge, >> struct drm_device *drm_dev = bridge->dev; >> int ret; >> >> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) >> - return 0; >> - >> - if (!bridge->encoder) { >> - DRM_ERROR("Missing encoder\n"); >> - return -ENODEV; >> - } >> - >> panel_bridge->link = device_link_add(drm_dev->dev, panel->dev, >> DL_FLAG_STATELESS); >> if (!panel_bridge->link) { >> @@ -83,6 +75,14 @@ static int panel_bridge_attach(struct drm_bridge >> *bridge, >> return -EINVAL; >> } >> >> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) >> + return 0; >> + >> + if (!bridge->encoder) { >> + DRM_ERROR("Missing encoder\n"); > > Shouldn't the device link be deleted in case of error? > Whoops! Yes, that's right. I'm sending a v2 right now. Thanks! Angelo > Regards, > Liu Ying > >> + return -ENODEV; >> + } >> + >> drm_connector_helper_add(connector, >> &panel_bridge_connector_helper_funcs); >> >> -- >> 2.42.0 >
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index e00d2e94c751..8c507dfd589e 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -67,14 +67,6 @@ static int panel_bridge_attach(struct drm_bridge *bridge, struct drm_device *drm_dev = bridge->dev; int ret; - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) - return 0; - - if (!bridge->encoder) { - DRM_ERROR("Missing encoder\n"); - return -ENODEV; - } - panel_bridge->link = device_link_add(drm_dev->dev, panel->dev, DL_FLAG_STATELESS); if (!panel_bridge->link) { @@ -83,6 +75,14 @@ static int panel_bridge_attach(struct drm_bridge *bridge, return -EINVAL; } + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) + return 0; + + if (!bridge->encoder) { + DRM_ERROR("Missing encoder\n"); + return -ENODEV; + } + drm_connector_helper_add(connector, &panel_bridge_connector_helper_funcs);
When external bridges are attached with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the panel bridge may also get the same flag, but in the .attach() callback for the panel bridge a device link is added only when this flag is not present; To make things worse, the .detach() callback tries to delete the device link unconditionally and without checking if it was created in the first place, crashing the kernel with a NULL pointer kernel panic upon calling panel_bridge_detach(). Fix that by moving the device_link_add() call before checking if the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is present. Fixes: 199cf07ebd2b ("drm/bridge: panel: Add a device link between drm device and panel device") Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/gpu/drm/bridge/panel.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)