From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web09.18257.1574150340178621727 for ; Mon, 18 Nov 2019 23:59:00 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Nov 2019 23:58:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,322,1569308400"; d="scan'208,217";a="231446226" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga004.fm.intel.com with ESMTP; 18 Nov 2019 23:58:59 -0800 Received: from fmsmsx162.amr.corp.intel.com (10.18.125.71) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 18 Nov 2019 23:58:59 -0800 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx162.amr.corp.intel.com (10.18.125.71) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 18 Nov 2019 23:58:58 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.127]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.200]) with mapi id 14.03.0439.000; Tue, 19 Nov 2019 15:58:57 +0800 From: "Ni, Ray" To: "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+pcGlRvkGaDXQkKoigIaeSIlCQ Date: Tue, 19 Nov 2019 07:58:56 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C364D29@SHSMSX104.ccr.corp.intel.com> References: In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_734D49CCEBEEF84792F5B80ED585239D5C364D29SHSMSX104ccrcor_" --_000_734D49CCEBEEF84792F5B80ED585239D5C364D29SHSMSX104ccrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable + 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_734D49CCEBEEF84792F5B80ED585239D5C364D29SHSMSX104ccrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

+ 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->Mode->MaxMode;<= /span>=

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_734D49CCEBEEF84792F5B80ED585239D5C364D29SHSMSX104ccrcor_--