public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* I found a fun bug in the Shell today. Looks like we have been getting lucky?
@ 2016-08-25  0:59 Andrew Fish
  2016-08-25  1:26 ` Andrew Fish
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Fish @ 2016-08-25  0:59 UTC (permalink / raw)
  To: edk2-devel

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? 

Thanks,

Andrew Fish

https://tianocore.acgmultimedia.com/show_bug.cgi?id=105


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-08-25 16:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-25  0:59 I found a fun bug in the Shell today. Looks like we have been getting lucky? Andrew Fish
2016-08-25  1:26 ` Andrew Fish
2016-08-25 16:05   ` Carsey, Jaben
2016-08-25 16:08     ` Andrew Fish
2016-08-25 16:17       ` Carsey, Jaben
2016-08-25 16:20         ` Andrew Fish

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox