From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DCA131A1E4B for ; Thu, 25 Aug 2016 09:06:26 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 25 Aug 2016 09:06:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,576,1464678000"; d="scan'208";a="871033687" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga003.jf.intel.com with ESMTP; 25 Aug 2016 09:06:01 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 25 Aug 2016 09:06:01 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.39]) by fmsmsx116.amr.corp.intel.com ([10.18.116.20]) with mapi id 14.03.0248.002; Thu, 25 Aug 2016 09:06:00 -0700 From: "Carsey, Jaben" To: Andrew Fish , edk2-devel CC: "Ni, Ruiyu" , "Carsey, Jaben" Thread-Topic: [edk2] I found a fun bug in the Shell today. Looks like we have been getting lucky? Thread-Index: AQHR/mwAbkxI5FMecUSc/Dw5klGU/qBZV+KAgACAPlA= Date: Thu, 25 Aug 2016 16:05:59 +0000 Message-ID: References: <095E0E05-A876-48C3-B87D-FA5874921821@apple.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTUyOWIwZjItMjllNC00MTcyLTg1OTEtM2Y0ZDliMDMyNTNjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Illvb05tNEFISFd1V1wvVUJKYjd1eU4wS2ZQdklCcGlYWmd4SmNid1JFdkVBPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [10.1.200.106] MIME-Version: 1.0 Subject: Re: I found a fun bug in the Shell today. Looks like we have been getting lucky? X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Aug 2016 16:06:27 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Andrew, Can you file a Bugzilla issue so we can track this issue properly? -Jaben > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Andrew Fish > Sent: Wednesday, August 24, 2016 6:26 PM > To: edk2-devel > Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we h= ave > been getting lucky? > Importance: High >=20 >=20 > > On Aug 24, 2016, at 5:59 PM, Andrew Fish wrote: > > > > I was tracking down a data corruption issue when paging was enabled on = an > edk2 shell command. The crash was in a custom ConSpliter over writing a D= XE > Core data structure. The buffer overflow seemed to be caused by the > Console getting confused on the location of the end of the screen. I set = a > watchpoint on gST->ConOut->Mode->CursorRow and found the shell was > the one corrupting the Mode data. > > > > UEFI Spec: The following data values in the SIMPLE_TEXT_OUTPUT_MODE > interface are read-only and are changed by using the appropriate interfac= e > functions: > > > > (master)>git grep "OurConOut.Mode" > > Application/Shell/ConsoleLogger.c:72: (*ConsoleInfo)->OurConOut.Mode > =3D gST->ConOut->Mode; > > Application/Shell/ConsoleLogger.c:647:// ShellInfoObject.ConsoleInfo= - > >OurConOut.Mode->CursorRow =3D 0; > > Application/Shell/ConsoleLogger.c:648:// ShellInfoObject.ConsoleInfo= - > >OurConOut.Mode->CursorColumn =3D 0; > > Application/Shell/ConsoleLogger.c:704: if (ConsoleInfo- > >OurConOut.Mode->CursorColumn > 0) { > > Application/Shell/ConsoleLogger.c:705: ConsoleInfo- > >OurConOut.Mode->CursorColumn--; > > Application/Shell/ConsoleLogger.c:734: ConsoleInfo->OurConOut.Mode= - > >CursorRow++; > > Application/Shell/ConsoleLogger.c:741: ConsoleInfo->OurConOut.Mode= - > >CursorColumn =3D 0; > > Application/Shell/ConsoleLogger.c:747: ConsoleInfo->OurConOut.Mode= - > >CursorColumn++; > > Application/Shell/ConsoleLogger.c:751: if ((INTN)ConsoleInfo- > >ColsPerScreen =3D=3D ConsoleInfo->OurConOut.Mode->CursorColumn + 1) { > > Application/Shell/ConsoleLogger.c:781: ConsoleInfo- > >OurConOut.Mode->CursorRow++; > > Application/Shell/ConsoleLogger.c:782: ConsoleInfo- > >OurConOut.Mode->CursorColumn =3D 0; > > Application/Shell/ConsoleLogger.c:976: ConsoleInfo->OurConOut.Mode = =3D > ConsoleInfo->OldConOut->Mode; > > > > > > I'm not exactly sure what this code is trying to do as the console shou= ld > update Mode structure directly? Maybe the intent was to have a copy of > gST->ConOut->Mode and keep it in sync? It seems like this should cause > more issues, but maybe the edk2 ConSplitter is not broken by this behavio= r > and we are getting lucky? > > >=20 > I forgot to mention that setting the Mode->CursorRow in the console code > back to the last row if was larger looks like it hides this bug in the sh= ell. >=20 > Thanks, >=20 > Andrew Fish >=20 >=20 > > Thanks, > > > > Andrew Fish > > > > https://tianocore.acgmultimedia.com/show_bug.cgi?id=3D105 > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel