From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Paul Subject: Re: [PATCH 2/3] drm/fb_helper: Fix references to dev->mode_config.num_connector Date: Thu, 05 May 2016 11:27:33 -0400 Message-ID: <1462462053.30874.3.camel@redhat.com> References: <1462375734-8213-1-git-send-email-cpaul@redhat.com> <1462375734-8213-2-git-send-email-cpaul@redhat.com> <20160504171135.GF1286@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20160504171135.GF1286@phenom.ffwll.local> Sender: stable-owner@vger.kernel.org To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, open list , stable@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org I would Cc it to stable just in case tbh. I picked up on this one since= KASAN was picking up on some of the memcpy() calls around here going out of b= ounds. I can include some of the backtraces from that if needed. On Wed, 2016-05-04 at 19:11 +0200, Daniel Vetter wrote: > On Wed, May 04, 2016 at 11:28:52AM -0400, Lyude wrote: > >=20 > > During boot, MST hotplugs are generally expected (even if no physic= al > > hotplugging occurs) and result in DRM's connector topology changing= =2E > > This means that using num_connector from the current mode configura= tion > > can lead to the number of connectors changing under us. This can le= ad to > > some nasty scenarios in fbcon: > >=20 > > - We allocate an array to the size of dev->mode_config.num_connecto= rs. > > - MST hotplug occurs, dev->mode_config.num_connectors gets incremen= ted. > > - We try to loop through each element in the array using the new va= lue > > =C2=A0 of dev->mode_config.num_connectors, and end up going out of = bounds > > =C2=A0 since dev->mode_config.num_connectors is now larger then the= array we > > =C2=A0 allocated. > >=20 > > fb_helper->connector_count however, will always remain consistent w= hile > > we do a modeset in fb_helper. > >=20 > > Cc: stable@vger.kernel.org > > Signed-off-by: Lyude > Ok, this one looks like it's indeed in the same critical section (wit= hin > drm_setup_outputs) as where we allocate the fb helper connector array= =2E So > I think this one here is not needed to fix a bug? >=20 > Still makes sense as a patch, but not with cc: stable I think. > -Daniel >=20 > >=20 > > --- > > =C2=A0drivers/gpu/drm/drm_fb_helper.c | 4 ++-- > > =C2=A01 file changed, 2 insertions(+), 2 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > b/drivers/gpu/drm/drm_fb_helper.c > > index 855108e..15204c0 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -1914,7 +1914,7 @@ static int drm_pick_crtcs(struct drm_fb_helpe= r > > *fb_helper, > > =C2=A0 if (modes[n] =3D=3D NULL) > > =C2=A0 return best_score; > > =C2=A0 > > - crtcs =3D kzalloc(dev->mode_config.num_connector * > > + crtcs =3D kzalloc(fb_helper->connector_count * > > =C2=A0 sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL); > > =C2=A0 if (!crtcs) > > =C2=A0 return best_score; > > @@ -1960,7 +1960,7 @@ static int drm_pick_crtcs(struct drm_fb_helpe= r > > *fb_helper, > > =C2=A0 if (score > best_score) { > > =C2=A0 best_score =3D score; > > =C2=A0 memcpy(best_crtcs, crtcs, > > - =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0dev->mode_config.num_= connector * > > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0fb_helper->connector_= count * > > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sizeof(struct dr= m_fb_helper_crtc *)); > > =C2=A0 } > > =C2=A0 } > > --=C2=A0 > > 2.5.5 > >=20 > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel --=20 Cheers, Lyude