From: Ilija Hadzic <ihadzic@research.bell-labs.com>
To: airlied@gmail.com, dri-devel@lists.freedesktop.org
Cc: Ilija Hadzic <ihadzic@research.bell-labs.com>
Subject: [PATCH 6/6] drm: eliminate bit-copy restoration of crtc
Date: Tue, 29 Oct 2013 11:09:46 -0400 [thread overview]
Message-ID: <1383059386-20917-7-git-send-email-ihadzic@research.bell-labs.com> (raw)
In-Reply-To: <1383059386-20917-1-git-send-email-ihadzic@research.bell-labs.com>
Bit-copying restoration of CRTC structure in failure-recovery
path of drm_crtc_helper_set_config function evokes a
subtle and rare, but very dangerous, corruption of
CRTC mutex structure.
Namely, if drm_crtc_helper_set_config takes the path under
'fail:' label *and* some other process has attempted to
grab the crtc mutex (and got blocked), restoring the CRTC
structure by bit-copying it will overwrite the CRTC mutex
state and the waiters list pointer within the mutex structure.
Consequently the blocked process will never be scheduled.
This patch fixes the issue by eliminating the bit-copy
restoration. The elimination is possible because previous
patches have cleaned up the resoration path so that only
the fields touched by the drm_crtc_helper_set_config function
are saved and restored if necessary.
Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
drivers/gpu/drm/drm_crtc_helper.c | 33 ++++++++-------------------------
1 file changed, 8 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index a1322af..d171032 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -596,7 +596,7 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
int drm_crtc_helper_set_config(struct drm_mode_set *set)
{
struct drm_device *dev;
- struct drm_crtc *save_crtcs, *new_crtc, *crtc;
+ struct drm_crtc *new_crtc;
struct drm_encoder *save_encoders, *new_encoder, *encoder;
bool mode_changed = false; /* if true do a full mode set */
bool fb_changed = false; /* if true and !mode_changed just do a flip */
@@ -633,38 +633,28 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
dev = set->crtc->dev;
- /* Allocate space for the backup of all (non-pointer) crtc, encoder and
- * connector data. */
- save_crtcs = kzalloc(dev->mode_config.num_crtc *
- sizeof(struct drm_crtc), GFP_KERNEL);
- if (!save_crtcs)
- return -ENOMEM;
-
+ /*
+ * Allocate space for the backup of all (non-pointer) encoder and
+ * connector data.
+ */
save_encoders = kzalloc(dev->mode_config.num_encoder *
sizeof(struct drm_encoder), GFP_KERNEL);
- if (!save_encoders) {
- kfree(save_crtcs);
+ if (!save_encoders)
return -ENOMEM;
- }
save_connectors = kzalloc(dev->mode_config.num_connector *
sizeof(struct drm_connector), GFP_KERNEL);
if (!save_connectors) {
- kfree(save_crtcs);
kfree(save_encoders);
return -ENOMEM;
}
- /* Copy data. Note that driver private data is not affected.
+ /*
+ * Copy data. Note that driver private data is not affected.
* Should anything bad happen only the expected state is
* restored, not the drivers personal bookkeeping.
*/
count = 0;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
- save_crtcs[count++] = *crtc;
- }
-
- count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
save_encoders[count++] = *encoder;
}
@@ -825,17 +815,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
kfree(save_connectors);
kfree(save_encoders);
- kfree(save_crtcs);
return 0;
fail:
/* Restore all previous data. */
count = 0;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
- *crtc = save_crtcs[count++];
- }
-
- count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
*encoder = save_encoders[count++];
}
@@ -853,7 +837,6 @@ fail:
kfree(save_connectors);
kfree(save_encoders);
- kfree(save_crtcs);
return ret;
}
EXPORT_SYMBOL(drm_crtc_helper_set_config);
--
1.8.2.1
next prev parent reply other threads:[~2013-10-29 15:21 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-29 15:09 fix for CRTC mutex corruption Ilija Hadzic
2013-10-29 15:09 ` [PATCH 1/6] drm: remove redundant if statement Ilija Hadzic
2013-10-29 15:09 ` [PATCH 2/6] drm: eliminate old_fb from drm_crtc_helper_set_config Ilija Hadzic
2013-10-29 15:09 ` [PATCH 3/6] drm: restore crtc origin if mode_set_base fails Ilija Hadzic
2013-10-29 15:09 ` [PATCH 4/6] drm: fix error recovery path in drm_crtc_helper_set_mode Ilija Hadzic
2013-10-29 15:09 ` [PATCH 5/6] drm: do not set crtc enabled field twice Ilija Hadzic
2013-10-29 15:09 ` Ilija Hadzic [this message]
2013-10-29 18:40 ` fix for CRTC mutex corruption Daniel Vetter
2013-10-29 19:22 ` Ilija Hadzic
2013-10-29 20:11 ` Daniel Vetter
2013-10-30 13:45 ` Ilija Hadzic
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=1383059386-20917-7-git-send-email-ihadzic@research.bell-labs.com \
--to=ihadzic@research.bell-labs.com \
--cc=airlied@gmail.com \
--cc=dri-devel@lists.freedesktop.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).