Re: I found a fun bug in the Shell today. Looks like we have been getting lucky?

["Carsey, Jaben" <jaben.carsey@intel.com>]

Doh!  Thanks!  I didn't realize that was the bug.  I will have to finally learn that servers name one of these days...

> -----Original Message-----
> From: afish@apple.com [mailto:afish@apple.com]
> Sent: Thursday, August 25, 2016 9:08 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>
> Cc: edk2-devel <edk2-devel@lists.01.org>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we have
> been getting lucky?
> Importance: High
> 
> 
> > On Aug 25, 2016, at 9:05 AM, Carsey, Jaben <jaben.carsey@intel.com>
> wrote:
> >
> > Andrew,
> >
> > Can you file a Bugzilla issue so we can track this issue properly?
> >
> 
> Jaben,
> 
> I attached the URL at the end of the original mail:
> https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
> 
> Thanks,
> 
> Andrew Fish
> 
> > -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 <edk2-devel@lists.01.org>
> >> Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we
> have
> >> been getting lucky?
> >> Importance: High
> >>
> >>
> >>> On Aug 24, 2016, at 5:59 PM, Andrew Fish <afish@apple.com> 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
> DXE
> >> 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
> interface
> >> functions:
> >>>
> >>> (master)>git grep "OurConOut.Mode"
> >>> Application/Shell/ConsoleLogger.c:72:  (*ConsoleInfo)-
> >OurConOut.Mode
> >> = gST->ConOut->Mode;
> >>> Application/Shell/ConsoleLogger.c:647://    ShellInfoObject.ConsoleInfo-
> >>> OurConOut.Mode->CursorRow    = 0;
> >>> Application/Shell/ConsoleLogger.c:648://    ShellInfoObject.ConsoleInfo-
> >>> OurConOut.Mode->CursorColumn = 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 = 0;
> >>> Application/Shell/ConsoleLogger.c:747:      ConsoleInfo-
> >OurConOut.Mode-
> >>> CursorColumn++;
> >>> Application/Shell/ConsoleLogger.c:751:      if ((INTN)ConsoleInfo-
> >>> ColsPerScreen == ConsoleInfo->OurConOut.Mode->CursorColumn + 1) {
> >>> Application/Shell/ConsoleLogger.c:781:        ConsoleInfo-
> >>> OurConOut.Mode->CursorRow++;
> >>> Application/Shell/ConsoleLogger.c:782:        ConsoleInfo-
> >>> OurConOut.Mode->CursorColumn = 0;
> >>> Application/Shell/ConsoleLogger.c:976:    ConsoleInfo-
> >OurConOut.Mode =
> >> ConsoleInfo->OldConOut->Mode;
> >>>
> >>>
> >>> I'm not exactly sure what this code is trying to do as the console should
> >> 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
> behavior
> >> and we are getting lucky?
> >>>
> >>
> >> 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 shell.
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>
> >>> Thanks,
> >>>
> >>> Andrew Fish
> >>>
> >>> https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
> >>> _______________________________________________
> >>> edk2-devel mailing list
> >>> edk2-devel@lists.01.org
> >>> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel