Re: [PATCH v2 i-g-t 1/4] igt_kms: Avoid NULL ptr deref when commiting disabled planes

Konduru, Chandra
2015-05-07T23:45:29Z

> >> diff --git a/lib/igt_kms.c b/lib/igt_kms.c index b7d1e90..33d437d
> >> 100644
> >> --- a/lib/igt_kms.c
> >> +++ b/lib/igt_kms.c
> >> @@ -1331,7 +1331,7 @@ static int igt_drm_plane_commit(igt_plane_t
> *plane,
> >> fb_id = igt_plane_get_fb_id(plane);
> >> crtc_id = output->config.crtc->crtc_id;
> >>
> >> - if (plane->fb_changed && fb_id == 0) {
> >> + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> >
> > Shouldn't this include plane->position_changed too? Like:
> > if ((plane->fb_changed || plane->size_changed ||
> > plane->position_changed) && fb_id == 0) {
>
> When you added size_changed, state for position_changed and fb == NULL
> remained the same, while size_changed added new state for size_changed ==
> true and fb == NULL. So I added handling for that and did not think much beyond
> it. It fixes a segfault so I moved on. Or in other words, I don't see how it would
> harm to merge this, it doesn't make anything worse.
Agree it doesn't harm to merge.
>
> Regards,
>
> Tvrtko