Commit 02ee4e94 authored by Ilija Hadzic's avatar Ilija Hadzic Committed by Dave Airlie

drm: eliminate bit-copy restoration of crtc

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: default avatarIlija Hadzic <ihadzic@research.bell-labs.com>
Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
parent 48b1f5dd
...@@ -604,7 +604,7 @@ drm_crtc_helper_disable(struct drm_crtc *crtc) ...@@ -604,7 +604,7 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
int drm_crtc_helper_set_config(struct drm_mode_set *set) int drm_crtc_helper_set_config(struct drm_mode_set *set)
{ {
struct drm_device *dev; 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; struct drm_encoder *save_encoders, *new_encoder, *encoder;
bool mode_changed = false; /* if true do a full mode set */ bool mode_changed = false; /* if true do a full mode set */
bool fb_changed = false; /* if true and !mode_changed just do a flip */ bool fb_changed = false; /* if true and !mode_changed just do a flip */
...@@ -641,37 +641,27 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) ...@@ -641,37 +641,27 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
dev = set->crtc->dev; dev = set->crtc->dev;
/* Allocate space for the backup of all (non-pointer) crtc, encoder and /*
* connector data. */ * Allocate space for the backup of all (non-pointer) encoder and
save_crtcs = kzalloc(dev->mode_config.num_crtc * * connector data.
sizeof(struct drm_crtc), GFP_KERNEL); */
if (!save_crtcs)
return -ENOMEM;
save_encoders = kzalloc(dev->mode_config.num_encoder * save_encoders = kzalloc(dev->mode_config.num_encoder *
sizeof(struct drm_encoder), GFP_KERNEL); sizeof(struct drm_encoder), GFP_KERNEL);
if (!save_encoders) { if (!save_encoders)
kfree(save_crtcs);
return -ENOMEM; return -ENOMEM;
}
save_connectors = kzalloc(dev->mode_config.num_connector * save_connectors = kzalloc(dev->mode_config.num_connector *
sizeof(struct drm_connector), GFP_KERNEL); sizeof(struct drm_connector), GFP_KERNEL);
if (!save_connectors) { if (!save_connectors) {
kfree(save_crtcs);
kfree(save_encoders); kfree(save_encoders);
return -ENOMEM; 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 * Should anything bad happen only the expected state is
* restored, not the drivers personal bookkeeping. * 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; count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
save_encoders[count++] = *encoder; save_encoders[count++] = *encoder;
...@@ -833,16 +823,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) ...@@ -833,16 +823,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
kfree(save_connectors); kfree(save_connectors);
kfree(save_encoders); kfree(save_encoders);
kfree(save_crtcs);
return 0; return 0;
fail: fail:
/* Restore all previous data. */ /* Restore all previous data. */
count = 0;
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
*crtc = save_crtcs[count++];
}
count = 0; count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
*encoder = save_encoders[count++]; *encoder = save_encoders[count++];
...@@ -861,7 +845,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) ...@@ -861,7 +845,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
kfree(save_connectors); kfree(save_connectors);
kfree(save_encoders); kfree(save_encoders);
kfree(save_crtcs);
return ret; return ret;
} }
EXPORT_SYMBOL(drm_crtc_helper_set_config); EXPORT_SYMBOL(drm_crtc_helper_set_config);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment