From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web12.24095.1574182001278701855 for ; Tue, 19 Nov 2019 08:46:41 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: michael.d.kinney@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Nov 2019 08:46:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,218,1571727600"; d="scan'208,217";a="237396851" Received: from orsmsx106.amr.corp.intel.com ([10.22.225.133]) by fmsmga002.fm.intel.com with ESMTP; 19 Nov 2019 08:46:37 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.200]) by ORSMSX106.amr.corp.intel.com ([169.254.1.150]) with mapi id 14.03.0439.000; Tue, 19 Nov 2019 08:46:37 -0800 From: "Michael D Kinney" To: "Ni, Ray" , "devel@edk2.groups.io" , "Gao, Zhichao" , "Kinney, Michael D" CC: Jonathan O'Neal Subject: Re: EDK2 bug report Thread-Topic: EDK2 bug report Thread-Index: AQHVnYjSnM+pcGlRvkGaDXQkKoigIaeSIlCQgACUOXA= Date: Tue, 19 Nov 2019 16:46:37 +0000 Message-ID: References: <734D49CCEBEEF84792F5B80ED585239D5C364D29@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C364D29@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.22.254.139] MIME-Version: 1.0 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_E92EE9817A31E24EB0585FDF735412F5B9E1FBE1ORSMSX113amrcor_" --_000_E92EE9817A31E24EB0585FDF735412F5B9E1FBE1ORSMSX113amrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ray, I agree that removing all UGA content makes the most sense. Mike From: Ni, Ray Sent: Monday, November 18, 2019 11:59 PM To: devel@edk2.groups.io; Gao, Zhichao ; Kinney, Mic= hael D Cc: Jonathan O'Neal Subject: RE: EDK2 bug report + Zhichao, Mike and mailing list. In general, if the code does contain bug handling UGA but isn't found until= now, I think we are safe to remove the UGA support now. From: Jonathan O'Neal > Sent: Monday, November 18, 2019 4:52 AM To: Ni, Ray > Subject: EDK2 bug report Hi - sorry to email you directly, but I couldn't find an address for a rele= vant mailing list, and I wasn't sure how else to report this. In trying to understand the EDK2 Graphics Console, I think I found an ugly = bug (which I might have also encountered in a UGA runtime environment). In= MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsole.c, GraphicsConsoleControllerDriverStart() does the following circa line 436: if (Private->GraphicsOutput !=3D NULL) { // console is GOP, handle as GOP // ... } else { // console is UGA Status =3D CheckModeSupported(Private->GraphicsOutput, ...); // !! note that Private->GraphicsOutput is known to be NULL here // ... } Then, in CheckModeSupported(), circa line 773, the GraphicsOutput pointer i= s dereferenced without a NULL check: MaxMode =3D GraphicsOutput->Mode->MaxMode; It appears that CheckModeSupported()only supports GOP, not UGA. Therefore,= GraphicsConsoleControllerDriverStart() should not be calling it when the o= utput device is known to be UGA - and/or, CheckModeSupported() should conta= in a defensive NULL check prior to dereferencing the GraphicsOutput pointer= . (I realize that UGA is deprecated, but if some level of UGA support remains= in the code, it seems like it shouldn't dereference a NULL pointer.) Sorry if emailing you directly was inappropriate; if it was, please let me= know whom I should contact. Thanks for your work on EDK. -Jonathan O'Neal --_000_E92EE9817A31E24EB0585FDF735412F5B9E1FBE1ORSMSX113amrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Ray,

 

I agree that removing all UGA content makes the most sense.<= o:p>

 

Mike

 

From: Ni, Ray <= ray.ni@intel.com>
Sent: Monday, November 18, 2019 11:59 PM
To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>= ; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Jonathan O'Neal <jko@piquantinnovation.com>
Subject: RE: EDK2 bug report

 

+ Zhichao, Mike= and mailing list.

 

In general, if the = code does contain bug handling UGA but isn’t found until now, I think= we are safe to remove the UGA support now.

 

 

From: Jonathan O'= Neal <jko@piquantinnovation= .com>
Sent: Monday, November 18, 2019 4:52 AM
To: Ni, Ray <ray.ni@intel.com= >
Subject: EDK2 bug report

 

= Hi - sorry to email you directly, but I couldn't find an address for a rele= vant mailing list, and I wasn't sure how else to report this.

In trying to understand the EDK2 Graphics Console, I think I found an ugly = bug (which I might have also encountered in a UGA runtime environment).&nbs= p; In MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsCons= ole.c,
GraphicsConsoleControllerDriverStart() does the following = circa line 436:

   if (Private->GraphicsOutput !=3D NULL) {
      // console is GOP, handle as GOP
      // ...
   } else {
      // console is UGA
      Status =3D CheckModeSupported(Private->Gr= aphicsOutput, ...);
      // !! note that Private->GraphicsOutput i= s known to be NULL here
      // ...
   }

= Then, in CheckModeSupported(), circa line 773, the GraphicsOutput pointer is dereferenced without a NULL check:

   MaxMode =3D GraphicsOutput->Mod= e->MaxMode;

It appears that CheckModeSupported()only supports GOP, not UGA.  Therefore, GraphicsConsoleControllerDrive= rStart() should not be calling it when the output device is known to be UGA - and/or, CheckModeSupported() should = contain a defensive NULL check prior to dereferencing the GraphicsOutput pointer.

= (I realize that UGA is deprecated, but if some level of UGA support remains= in the code, it seems like it shouldn't dereference a NULL pointer.)

= Sorry if emailing you directly was inappropriate;  if it was, please l= et me know whom I should contact.

= Thanks for your work on EDK.

-Jonathan O'Neal

--_000_E92EE9817A31E24EB0585FDF735412F5B9E1FBE1ORSMSX113amrcor_--