From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757444AbcEEP1g (ORCPT ); Thu, 5 May 2016 11:27:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54902 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754807AbcEEP1f (ORCPT ); Thu, 5 May 2016 11:27:35 -0400 Message-ID: <1462462053.30874.3.camel@redhat.com> Subject: Re: [PATCH 2/3] drm/fb_helper: Fix references to dev->mode_config.num_connector From: Lyude Paul To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, open list , stable@vger.kernel.org Date: Thu, 05 May 2016 11:27:33 -0400 In-Reply-To: <20160504171135.GF1286@phenom.ffwll.local> References: <1462375734-8213-1-git-send-email-cpaul@redhat.com> <1462375734-8213-2-git-send-email-cpaul@redhat.com> <20160504171135.GF1286@phenom.ffwll.local> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 bounds. 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: > > > > During boot, MST hotplugs are generally expected (even if no physical > > hotplugging occurs) and result in DRM's connector topology changing. > > This means that using num_connector from the current mode configuration > > can lead to the number of connectors changing under us. This can lead to > > some nasty scenarios in fbcon: > > > > - We allocate an array to the size of dev->mode_config.num_connectors. > > - MST hotplug occurs, dev->mode_config.num_connectors gets incremented. > > - We try to loop through each element in the array using the new value > >   of dev->mode_config.num_connectors, and end up going out of bounds > >   since dev->mode_config.num_connectors is now larger then the array we > >   allocated. > > > > fb_helper->connector_count however, will always remain consistent while > > we do a modeset in fb_helper. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Lyude > Ok, this one looks like it's indeed in the same critical section (within > drm_setup_outputs) as where we allocate the fb helper connector array. So > I think this one here is not needed to fix a bug? > > Still makes sense as a patch, but not with cc: stable I think. > -Daniel > > > > > --- > >  drivers/gpu/drm/drm_fb_helper.c | 4 ++-- > >  1 file changed, 2 insertions(+), 2 deletions(-) > > > > 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_helper > > *fb_helper, > >   if (modes[n] == NULL) > >   return best_score; > >   > > - crtcs = kzalloc(dev->mode_config.num_connector * > > + crtcs = kzalloc(fb_helper->connector_count * > >   sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL); > >   if (!crtcs) > >   return best_score; > > @@ -1960,7 +1960,7 @@ static int drm_pick_crtcs(struct drm_fb_helper > > *fb_helper, > >   if (score > best_score) { > >   best_score = score; > >   memcpy(best_crtcs, crtcs, > > -        dev->mode_config.num_connector * > > +        fb_helper->connector_count * > >          sizeof(struct drm_fb_helper_crtc *)); > >   } > >   } > > --  > > 2.5.5 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Cheers, Lyude