| Message ID | 20220218100403.7028-8-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Review of mode copies | expand |
On Fri, Feb 18, 2022 at 11:04 AM Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > struct drm_display_mode embeds a list head, so overwriting > the full struct with another one will corrupt the list > (if the destination mode is on a list). Use drm_mode_copy() > instead which explicitly preserves the list head of > the destination mode. > > Even if we know the destination mode is not on any list > using drm_mode_copy() seems decent as it sets a good > example. Bad examples of not using it might eventually > get copied into code where preserving the list head > actually matters. > > Obviously one case not covered here is when the mode > itself is embedded in a larger structure and the whole > structure is copied. But if we are careful when copying > into modes embedded in structures I think we can be a > little more reassured that bogus list heads haven't been > propagated in. > > @is_mode_copy@ > @@ > drm_mode_copy(...) > { > ... > } > > @depends on !is_mode_copy@ > struct drm_display_mode *mode; > expression E, S; > @@ > ( > - *mode = E > + drm_mode_copy(mode, &E) > | > - memcpy(mode, E, S) > + drm_mode_copy(mode, E) > ) > > @depends on !is_mode_copy@ > struct drm_display_mode mode; > expression E; > @@ > ( > - mode = E > + drm_mode_copy(&mode, &E) > | > - memcpy(&mode, E, S) > + drm_mode_copy(&mode, E) > ) > > @@ > struct drm_display_mode *mode; > @@ > - &*mode > + mode > > Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Looks good. Thanks! Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > --- > drivers/gpu/drm/gma500/oaktrail_crtc.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c > index 36c7c2686c90..79fc602b35bc 100644 > --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c > +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c > @@ -385,12 +385,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > if (!gma_power_begin(dev, true)) > return 0; > > - memcpy(&gma_crtc->saved_mode, > - mode, > - sizeof(struct drm_display_mode)); > - memcpy(&gma_crtc->saved_adjusted_mode, > - adjusted_mode, > - sizeof(struct drm_display_mode)); > + drm_mode_copy(&gma_crtc->saved_mode, mode); > + drm_mode_copy(&gma_crtc->saved_adjusted_mode, adjusted_mode); > > list_for_each_entry(connector, &mode_config->connector_list, head) { > if (!connector->encoder || connector->encoder->crtc != crtc) > -- > 2.34.1 >
diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c index 36c7c2686c90..79fc602b35bc 100644 --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c @@ -385,12 +385,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, if (!gma_power_begin(dev, true)) return 0; - memcpy(&gma_crtc->saved_mode, - mode, - sizeof(struct drm_display_mode)); - memcpy(&gma_crtc->saved_adjusted_mode, - adjusted_mode, - sizeof(struct drm_display_mode)); + drm_mode_copy(&gma_crtc->saved_mode, mode); + drm_mode_copy(&gma_crtc->saved_adjusted_mode, adjusted_mode); list_for_each_entry(connector, &mode_config->connector_list, head) { if (!connector->encoder || connector->encoder->crtc != crtc)