From: Paul Cercueil <paul@crapouillou.net> To: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@linux.ie>, Sam Ravnborg <sam@ravnborg.org>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, od@zcrc.me, dri-devel <dri-devel@lists.freedesktop.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, stable <stable@vger.kernel.org>, Andrzej Hajda <a.hajda@samsung.com>, Neil Armstrong <narmstrong@baylibre.com>, Jonas Karlman <jonas@kwiboo.se>, Jernej Skrabec <jernej.skrabec@siol.net> Subject: Re: [PATCH v2 1/3] drm: bridge/panel: Cleanup connector on bridge detach Date: Wed, 20 Jan 2021 16:25:16 +0000 [thread overview] Message-ID: <4YQ8NQ.HNQ7IMBKVEBV2@crapouillou.net> (raw) In-Reply-To: <CAKMK7uGGDe8bZpeTnyCkF7g_2gC1nixOzWe4FWYXPRWi-q5y7A@mail.gmail.com> Le mer. 20 janv. 2021 à 17:03, Daniel Vetter <daniel@ffwll.ch> a écrit : > On Wed, Jan 20, 2021 at 1:35 PM Paul Cercueil <paul@crapouillou.net> > wrote: >> >> If we don't call drm_connector_cleanup() manually in >> panel_bridge_detach(), the connector will be cleaned up with the >> other >> DRM objects in the call to drm_mode_config_cleanup(). However, >> since our >> drm_connector is devm-allocated, by the time >> drm_mode_config_cleanup() >> will be called, our connector will be long gone. Therefore, the >> connector must be cleaned up when the bridge is detached to avoid >> use-after-free conditions. > > For -fixes this sounds ok, but for -next I think switching to drmm_ > would be much better. The API would need to change to have access to the drm_device struct, though. That would be quite a big patch, there are a few dozens source files that use this API already. Cheers, -Paul > >> v2: Cleanup connector only if it was created >> >> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper >> from the lvds-encoder bridge.") >> Cc: <stable@vger.kernel.org> # 4.12+ >> Cc: Andrzej Hajda <a.hajda@samsung.com> >> Cc: Neil Armstrong <narmstrong@baylibre.com> >> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> >> Cc: Jonas Karlman <jonas@kwiboo.se> >> Cc: Jernej Skrabec <jernej.skrabec@siol.net> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> drivers/gpu/drm/bridge/panel.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/panel.c >> b/drivers/gpu/drm/bridge/panel.c >> index 0ddc37551194..df86b0ee0549 100644 >> --- a/drivers/gpu/drm/bridge/panel.c >> +++ b/drivers/gpu/drm/bridge/panel.c >> @@ -87,6 +87,12 @@ static int panel_bridge_attach(struct drm_bridge >> *bridge, >> >> static void panel_bridge_detach(struct drm_bridge *bridge) >> { >> + struct panel_bridge *panel_bridge = >> drm_bridge_to_panel_bridge(bridge); >> + struct drm_connector *connector = &panel_bridge->connector; >> + >> + /* Cleanup the connector if we know it was initialized */ >> + if (!!panel_bridge->connector.dev) >> + drm_connector_cleanup(connector); >> } >> >> static void panel_bridge_pre_enable(struct drm_bridge *bridge) >> -- >> 2.29.2 >> > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
WARNING: multiple messages have this Message-ID (diff)
From: Paul Cercueil <paul@crapouillou.net> To: Daniel Vetter <daniel@ffwll.ch> Cc: Jernej Skrabec <jernej.skrabec@siol.net>, Neil Armstrong <narmstrong@baylibre.com>, David Airlie <airlied@linux.ie>, Jonas Karlman <jonas@kwiboo.se>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Andrzej Hajda <a.hajda@samsung.com>, od@zcrc.me, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, stable <stable@vger.kernel.org>, Sam Ravnborg <sam@ravnborg.org> Subject: Re: [PATCH v2 1/3] drm: bridge/panel: Cleanup connector on bridge detach Date: Wed, 20 Jan 2021 16:25:16 +0000 [thread overview] Message-ID: <4YQ8NQ.HNQ7IMBKVEBV2@crapouillou.net> (raw) In-Reply-To: <CAKMK7uGGDe8bZpeTnyCkF7g_2gC1nixOzWe4FWYXPRWi-q5y7A@mail.gmail.com> Le mer. 20 janv. 2021 à 17:03, Daniel Vetter <daniel@ffwll.ch> a écrit : > On Wed, Jan 20, 2021 at 1:35 PM Paul Cercueil <paul@crapouillou.net> > wrote: >> >> If we don't call drm_connector_cleanup() manually in >> panel_bridge_detach(), the connector will be cleaned up with the >> other >> DRM objects in the call to drm_mode_config_cleanup(). However, >> since our >> drm_connector is devm-allocated, by the time >> drm_mode_config_cleanup() >> will be called, our connector will be long gone. Therefore, the >> connector must be cleaned up when the bridge is detached to avoid >> use-after-free conditions. > > For -fixes this sounds ok, but for -next I think switching to drmm_ > would be much better. The API would need to change to have access to the drm_device struct, though. That would be quite a big patch, there are a few dozens source files that use this API already. Cheers, -Paul > >> v2: Cleanup connector only if it was created >> >> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper >> from the lvds-encoder bridge.") >> Cc: <stable@vger.kernel.org> # 4.12+ >> Cc: Andrzej Hajda <a.hajda@samsung.com> >> Cc: Neil Armstrong <narmstrong@baylibre.com> >> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> >> Cc: Jonas Karlman <jonas@kwiboo.se> >> Cc: Jernej Skrabec <jernej.skrabec@siol.net> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> drivers/gpu/drm/bridge/panel.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/panel.c >> b/drivers/gpu/drm/bridge/panel.c >> index 0ddc37551194..df86b0ee0549 100644 >> --- a/drivers/gpu/drm/bridge/panel.c >> +++ b/drivers/gpu/drm/bridge/panel.c >> @@ -87,6 +87,12 @@ static int panel_bridge_attach(struct drm_bridge >> *bridge, >> >> static void panel_bridge_detach(struct drm_bridge *bridge) >> { >> + struct panel_bridge *panel_bridge = >> drm_bridge_to_panel_bridge(bridge); >> + struct drm_connector *connector = &panel_bridge->connector; >> + >> + /* Cleanup the connector if we know it was initialized */ >> + if (!!panel_bridge->connector.dev) >> + drm_connector_cleanup(connector); >> } >> >> static void panel_bridge_pre_enable(struct drm_bridge *bridge) >> -- >> 2.29.2 >> > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-01-20 17:16 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-20 12:35 [PATCH v2 0/3] Fixes to bridge/panel and ingenic-drm Paul Cercueil 2021-01-20 12:35 ` Paul Cercueil 2021-01-20 12:35 ` [PATCH v2 1/3] drm: bridge/panel: Cleanup connector on bridge detach Paul Cercueil 2021-01-20 12:35 ` Paul Cercueil 2021-01-20 16:03 ` Daniel Vetter 2021-01-20 16:03 ` Daniel Vetter 2021-01-20 16:25 ` Paul Cercueil [this message] 2021-01-20 16:25 ` Paul Cercueil 2021-01-20 17:38 ` Daniel Vetter 2021-01-20 17:38 ` Daniel Vetter 2021-03-24 2:15 ` Laurent Pinchart 2021-03-24 2:15 ` Laurent Pinchart 2021-03-24 9:39 ` Daniel Vetter 2021-03-24 9:39 ` Daniel Vetter 2021-03-24 9:46 ` Laurent Pinchart 2021-03-24 9:46 ` Laurent Pinchart 2021-01-20 12:35 ` [PATCH v2 2/3] drm/ingenic: Register devm action to cleanup encoders Paul Cercueil 2021-01-20 12:35 ` Paul Cercueil 2021-01-20 13:01 ` Daniel Vetter 2021-01-20 13:01 ` Daniel Vetter 2021-01-20 13:21 ` Paul Cercueil 2021-01-20 13:21 ` Paul Cercueil 2021-01-20 14:04 ` Daniel Vetter 2021-01-20 14:04 ` Daniel Vetter 2021-01-20 15:55 ` Paul Cercueil 2021-01-20 15:55 ` Paul Cercueil 2021-01-20 12:35 ` [PATCH v2 3/3] drm/ingenic: Fix non-OSD mode Paul Cercueil 2021-01-20 12:35 ` Paul Cercueil 2021-01-20 16:26 ` Daniel Vetter 2021-01-20 16:26 ` Daniel Vetter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4YQ8NQ.HNQ7IMBKVEBV2@crapouillou.net \ --to=paul@crapouillou.net \ --cc=a.hajda@samsung.com \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=jernej.skrabec@siol.net \ --cc=jonas@kwiboo.se \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-kernel@vger.kernel.org \ --cc=narmstrong@baylibre.com \ --cc=od@zcrc.me \ --cc=sam@ravnborg.org \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.