* [PATCH] Fix edit on screens with more than 200 columns @ 2017-01-24 13:13 Witt, Sebastian 2017-01-24 16:27 ` Carsey, Jaben 0 siblings, 1 reply; 6+ messages in thread From: Witt, Sebastian @ 2017-01-24 13:13 UTC (permalink / raw) To: edk2-devel@lists.01.org If the shell edit command is used on a screen with more than 200 columns, we get a buffer overflow. This increases the default buffer size to 400 columns and allocates a pool when this is not enough. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> --- .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c index 6ebf002..d81dd01 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c @@ -302,12 +302,21 @@ EditorClearLine ( IN UINTN LastRow ) { - CHAR16 Line[200]; + CHAR16 Buffer[400]; + CHAR16 *Line = Buffer; if (Row == 0) { Row = 1; } + // If there are more columns than our buffer can contain, allocate new buffer + if (LastCol >= (sizeof (Buffer) / sizeof (CHAR16))) { + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); + if (Line == NULL) { + return; + } + } + // // prepare a blank line // @@ -326,6 +335,10 @@ EditorClearLine ( // print out the blank line // ShellPrintEx (0, ((INT32)Row) - 1, Line); + + // Free if allocated + if (Line != Buffer) + FreePool (Line); } /** -- 2.1.4 With best regards, Sebastian Witt Siemens AG Digital Factory Division Factory Automation Automation Products and Systems DF FA AS DH KHE 1 Oestliche Rheinbrueckenstr. 50 76187 Karlsruhe, Germany Tel.: +49 721 595-5326 mailto:sebastian.witt@siemens.com www.siemens.com/ingenuityforlife Siemens Aktiengesellschaft: Chairman of the Supervisory Board: Gerhard Cromme; Managing Board: Joe Kaeser, Chairman, President and Chief Executive Officer; Roland Busch, Lisa Davis, Klaus Helmrich, Janina Kugel, Siegfried Russwurm, Ralf P. Thomas; Registered offices: Berlin and Munich, Germany; Commercial registries: Berlin Charlottenburg, HRB 12300, Munich, HRB 6684; WEEE-Reg.-No. DE 23691322 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix edit on screens with more than 200 columns 2017-01-24 13:13 [PATCH] Fix edit on screens with more than 200 columns Witt, Sebastian @ 2017-01-24 16:27 ` Carsey, Jaben 2017-01-24 16:47 ` Witt, Sebastian 0 siblings, 1 reply; 6+ messages in thread From: Carsey, Jaben @ 2017-01-24 16:27 UTC (permalink / raw) To: Witt, Sebastian, edk2-devel@lists.01.org; +Cc: Carsey, Jaben Is there a reason to not just always start with allocating the 400 and then we don't need to complicate the end to conditionally free the buffer? > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Witt, > Sebastian > Sent: Tuesday, January 24, 2017 5:14 AM > To: edk2-devel@lists.01.org > Subject: [edk2] [PATCH] Fix edit on screens with more than 200 columns > Importance: High > > If the shell edit command is used on a screen with more than > 200 columns, we get a buffer overflow. This increases the default buffer size to > 400 columns and allocates a pool when this is not enough. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> > > --- > .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLi > b.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLi > b.c > index 6ebf002..d81dd01 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLi > b.c > +++ > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Command > +++ sLib.c > @@ -302,12 +302,21 @@ EditorClearLine ( > IN UINTN LastRow > ) > { > - CHAR16 Line[200]; > + CHAR16 Buffer[400]; > + CHAR16 *Line = Buffer; > > if (Row == 0) { > Row = 1; > } > > + // If there are more columns than our buffer can contain, allocate > + new buffer if (LastCol >= (sizeof (Buffer) / sizeof (CHAR16))) { > + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); > + if (Line == NULL) { > + return; > + } > + } > + > // > // prepare a blank line > // > @@ -326,6 +335,10 @@ EditorClearLine ( > // print out the blank line > // > ShellPrintEx (0, ((INT32)Row) - 1, Line); > + > + // Free if allocated > + if (Line != Buffer) > + FreePool (Line); > } > > /** > -- > 2.1.4 > > With best regards, > Sebastian Witt > > Siemens AG > Digital Factory Division > Factory Automation > Automation Products and Systems > DF FA AS DH KHE 1 > Oestliche Rheinbrueckenstr. 50 > 76187 Karlsruhe, Germany > Tel.: +49 721 595-5326 > mailto:sebastian.witt@siemens.com > > www.siemens.com/ingenuityforlife > > Siemens Aktiengesellschaft: Chairman of the Supervisory Board: Gerhard > Cromme; Managing Board: Joe Kaeser, Chairman, President and Chief Executive > Officer; Roland Busch, Lisa Davis, Klaus Helmrich, Janina Kugel, Siegfried > Russwurm, Ralf P. Thomas; Registered offices: Berlin and Munich, Germany; > Commercial registries: Berlin Charlottenburg, HRB 12300, Munich, HRB 6684; > WEEE-Reg.-No. DE 23691322 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix edit on screens with more than 200 columns 2017-01-24 16:27 ` Carsey, Jaben @ 2017-01-24 16:47 ` Witt, Sebastian 2017-01-24 17:36 ` Carsey, Jaben 0 siblings, 1 reply; 6+ messages in thread From: Witt, Sebastian @ 2017-01-24 16:47 UTC (permalink / raw) To: Carsey, Jaben, edk2-devel@lists.01.org Only performance. But I haven't measured if there is a big difference between static buffer and AllocateZeroPool. I wouldn't use a fixed value. There may be a display device with more than 400 columns. Otherwise one can always allocate the [LastCol + 1] buffer. -----Original Message----- From: Carsey, Jaben [mailto:jaben.carsey@intel.com] Sent: Dienstag, 24. Januar 2017 17:27 To: Witt, Sebastian (DF FA AS DH KHE 1); edk2-devel@lists.01.org Cc: Carsey, Jaben Subject: RE: [PATCH] Fix edit on screens with more than 200 columns Is there a reason to not just always start with allocating the 400 and then we don't need to complicate the end to conditionally free the buffer? > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Witt, Sebastian > Sent: Tuesday, January 24, 2017 5:14 AM > To: edk2-devel@lists.01.org > Subject: [edk2] [PATCH] Fix edit on screens with more than 200 columns > Importance: High > > If the shell edit command is used on a screen with more than > 200 columns, we get a buffer overflow. This increases the default > buffer size to > 400 columns and allocates a pool when this is not enough. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> > > --- > .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsL > i > b.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsL > i > b.c > index 6ebf002..d81dd01 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsL > i > b.c > +++ > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Command > +++ sLib.c > @@ -302,12 +302,21 @@ EditorClearLine ( > IN UINTN LastRow > ) > { > - CHAR16 Line[200]; > + CHAR16 Buffer[400]; > + CHAR16 *Line = Buffer; > > if (Row == 0) { > Row = 1; > } > > + // If there are more columns than our buffer can contain, allocate > + new buffer if (LastCol >= (sizeof (Buffer) / sizeof (CHAR16))) { > + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); > + if (Line == NULL) { > + return; > + } > + } > + > // > // prepare a blank line > // > @@ -326,6 +335,10 @@ EditorClearLine ( > // print out the blank line > // > ShellPrintEx (0, ((INT32)Row) - 1, Line); > + > + // Free if allocated > + if (Line != Buffer) > + FreePool (Line); > } > > /** > -- > 2.1.4 > > With best regards, > Sebastian Witt > > Siemens AG > Digital Factory Division > Factory Automation > Automation Products and Systems > DF FA AS DH KHE 1 > Oestliche Rheinbrueckenstr. 50 > 76187 Karlsruhe, Germany > Tel.: +49 721 595-5326 > mailto:sebastian.witt@siemens.com > > www.siemens.com/ingenuityforlife > > Siemens Aktiengesellschaft: Chairman of the Supervisory Board: Gerhard > Cromme; Managing Board: Joe Kaeser, Chairman, President and Chief > Executive Officer; Roland Busch, Lisa Davis, Klaus Helmrich, Janina > Kugel, Siegfried Russwurm, Ralf P. Thomas; Registered offices: Berlin > and Munich, Germany; Commercial registries: Berlin Charlottenburg, HRB > 12300, Munich, HRB 6684; WEEE-Reg.-No. DE 23691322 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix edit on screens with more than 200 columns 2017-01-24 16:47 ` Witt, Sebastian @ 2017-01-24 17:36 ` Carsey, Jaben 2017-01-26 9:00 ` Witt, Sebastian 0 siblings, 1 reply; 6+ messages in thread From: Carsey, Jaben @ 2017-01-24 17:36 UTC (permalink / raw) To: Witt, Sebastian, edk2-devel@lists.01.org; +Cc: Carsey, Jaben I didn't mean a 400 static, I meant start by allocating 400 and simplify the end of the function... > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Witt, Sebastian > Sent: Tuesday, January 24, 2017 8:48 AM > To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH] Fix edit on screens with more than 200 columns > Importance: High > > Only performance. But I haven't measured if there is a big difference > between static buffer and AllocateZeroPool. I wouldn't use a fixed value. > There may be a display device with more than 400 columns. Otherwise one > can always allocate the [LastCol + 1] buffer. > > -----Original Message----- > From: Carsey, Jaben [mailto:jaben.carsey@intel.com] > Sent: Dienstag, 24. Januar 2017 17:27 > To: Witt, Sebastian (DF FA AS DH KHE 1); edk2-devel@lists.01.org > Cc: Carsey, Jaben > Subject: RE: [PATCH] Fix edit on screens with more than 200 columns > > Is there a reason to not just always start with allocating the 400 and then we > don't need to complicate the end to conditionally free the buffer? > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > > Witt, Sebastian > > Sent: Tuesday, January 24, 2017 5:14 AM > > To: edk2-devel@lists.01.org > > Subject: [edk2] [PATCH] Fix edit on screens with more than 200 columns > > Importance: High > > > > If the shell edit command is used on a screen with more than > > 200 columns, we get a buffer overflow. This increases the default > > buffer size to > > 400 columns and allocates a pool when this is not enough. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> > > > > --- > > .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git > > > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsL > > i > > b.c > > > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsL > > i > > b.c > > index 6ebf002..d81dd01 100644 > > --- > > > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsL > > i > > b.c > > +++ > > > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > d > > +++ sLib.c > > @@ -302,12 +302,21 @@ EditorClearLine ( > > IN UINTN LastRow > > ) > > { > > - CHAR16 Line[200]; > > + CHAR16 Buffer[400]; > > + CHAR16 *Line = Buffer; > > > > if (Row == 0) { > > Row = 1; > > } > > > > + // If there are more columns than our buffer can contain, allocate > > + new buffer if (LastCol >= (sizeof (Buffer) / sizeof (CHAR16))) { > > + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); > > + if (Line == NULL) { > > + return; > > + } > > + } > > + > > // > > // prepare a blank line > > // > > @@ -326,6 +335,10 @@ EditorClearLine ( > > // print out the blank line > > // > > ShellPrintEx (0, ((INT32)Row) - 1, Line); > > + > > + // Free if allocated > > + if (Line != Buffer) > > + FreePool (Line); > > } > > > > /** > > -- > > 2.1.4 > > > > With best regards, > > Sebastian Witt > > > > Siemens AG > > Digital Factory Division > > Factory Automation > > Automation Products and Systems > > DF FA AS DH KHE 1 > > Oestliche Rheinbrueckenstr. 50 > > 76187 Karlsruhe, Germany > > Tel.: +49 721 595-5326 > > mailto:sebastian.witt@siemens.com > > > > www.siemens.com/ingenuityforlife > > > > Siemens Aktiengesellschaft: Chairman of the Supervisory Board: Gerhard > > Cromme; Managing Board: Joe Kaeser, Chairman, President and Chief > > Executive Officer; Roland Busch, Lisa Davis, Klaus Helmrich, Janina > > Kugel, Siegfried Russwurm, Ralf P. Thomas; Registered offices: Berlin > > and Munich, Germany; Commercial registries: Berlin Charlottenburg, HRB > > 12300, Munich, HRB 6684; WEEE-Reg.-No. DE 23691322 > > _______________________________________________ > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix edit on screens with more than 200 columns 2017-01-24 17:36 ` Carsey, Jaben @ 2017-01-26 9:00 ` Witt, Sebastian 2017-01-26 17:05 ` Carsey, Jaben 0 siblings, 1 reply; 6+ messages in thread From: Witt, Sebastian @ 2017-01-26 9:00 UTC (permalink / raw) To: Carsey, Jaben, edk2-devel@lists.01.org Like this? If the shell edit command is used on a screen with more than 200 columns, we get a buffer overflow. This always allocates a pool for the columns. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> --- .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c index 6ebf002..d81dd01 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c @@ -302,12 +302,21 @@ EditorClearLine ( IN UINTN LastRow ) { - CHAR16 Line[200]; + CHAR16 *Line; if (Row == 0) { Row = 1; } + // Allocate buffer for columns + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); + if (Line == NULL) { + return; + } + // // prepare a blank line // @@ -326,6 +335,10 @@ EditorClearLine ( // print out the blank line // ShellPrintEx (0, ((INT32)Row) - 1, Line); + + FreePool (Line); } /** -- 2.1.4 -----Original Message----- From: Carsey, Jaben [mailto:jaben.carsey@intel.com] Sent: Dienstag, 24. Januar 2017 18:36 To: Witt, Sebastian (DF FA AS DH KHE 1); edk2-devel@lists.01.org Cc: Carsey, Jaben Subject: RE: [PATCH] Fix edit on screens with more than 200 columns I didn't mean a 400 static, I meant start by allocating 400 and simplify the end of the function... > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Witt, Sebastian > Sent: Tuesday, January 24, 2017 8:48 AM > To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH] Fix edit on screens with more than 200 > columns > Importance: High > > Only performance. But I haven't measured if there is a big difference > between static buffer and AllocateZeroPool. I wouldn't use a fixed value. > There may be a display device with more than 400 columns. Otherwise > one can always allocate the [LastCol + 1] buffer. > > -----Original Message----- > From: Carsey, Jaben [mailto:jaben.carsey@intel.com] > Sent: Dienstag, 24. Januar 2017 17:27 > To: Witt, Sebastian (DF FA AS DH KHE 1); edk2-devel@lists.01.org > Cc: Carsey, Jaben > Subject: RE: [PATCH] Fix edit on screens with more than 200 columns > > Is there a reason to not just always start with allocating the 400 and > then we don't need to complicate the end to conditionally free the buffer? > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > > Of Witt, Sebastian > > Sent: Tuesday, January 24, 2017 5:14 AM > > To: edk2-devel@lists.01.org > > Subject: [edk2] [PATCH] Fix edit on screens with more than 200 > > columns > > Importance: High > > > > If the shell edit command is used on a screen with more than > > 200 columns, we get a buffer overflow. This increases the default > > buffer size to > > 400 columns and allocates a pool when this is not enough. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> > > > > --- > > .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git > > > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsL > > i > > b.c > > > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsL > > i > > b.c > > index 6ebf002..d81dd01 100644 > > --- > > > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsL > > i > > b.c > > +++ > > > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > d > > +++ sLib.c > > @@ -302,12 +302,21 @@ EditorClearLine ( > > IN UINTN LastRow > > ) > > { > > - CHAR16 Line[200]; > > + CHAR16 Buffer[400]; > > + CHAR16 *Line = Buffer; > > > > if (Row == 0) { > > Row = 1; > > } > > > > + // If there are more columns than our buffer can contain, > > + allocate new buffer if (LastCol >= (sizeof (Buffer) / sizeof (CHAR16))) { > > + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); > > + if (Line == NULL) { > > + return; > > + } > > + } > > + > > // > > // prepare a blank line > > // > > @@ -326,6 +335,10 @@ EditorClearLine ( > > // print out the blank line > > // > > ShellPrintEx (0, ((INT32)Row) - 1, Line); > > + > > + // Free if allocated > > + if (Line != Buffer) > > + FreePool (Line); > > } > > > > /** > > -- > > 2.1.4 > > > > With best regards, > > Sebastian Witt > > > > Siemens AG > > Digital Factory Division > > Factory Automation > > Automation Products and Systems > > DF FA AS DH KHE 1 > > Oestliche Rheinbrueckenstr. 50 > > 76187 Karlsruhe, Germany > > Tel.: +49 721 595-5326 > > mailto:sebastian.witt@siemens.com > > > > www.siemens.com/ingenuityforlife > > > > Siemens Aktiengesellschaft: Chairman of the Supervisory Board: > > Gerhard Cromme; Managing Board: Joe Kaeser, Chairman, President and > > Chief Executive Officer; Roland Busch, Lisa Davis, Klaus Helmrich, > > Janina Kugel, Siegfried Russwurm, Ralf P. Thomas; Registered > > offices: Berlin and Munich, Germany; Commercial registries: Berlin > > Charlottenburg, HRB 12300, Munich, HRB 6684; WEEE-Reg.-No. DE > > 23691322 _______________________________________________ > > 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix edit on screens with more than 200 columns 2017-01-26 9:00 ` Witt, Sebastian @ 2017-01-26 17:05 ` Carsey, Jaben 0 siblings, 0 replies; 6+ messages in thread From: Carsey, Jaben @ 2017-01-26 17:05 UTC (permalink / raw) To: Witt, Sebastian, edk2-devel@lists.01.org, Ni, Ruiyu; +Cc: Carsey, Jaben That seems good to me. Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> Ray, what do you think? -Jaben > -----Original Message----- > From: Witt, Sebastian [mailto:sebastian.witt@siemens.com] > Sent: Thursday, January 26, 2017 1:01 AM > To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org > Subject: RE: [PATCH] Fix edit on screens with more than 200 columns > Importance: High > > Like this? > > If the shell edit command is used on a screen with more than > 200 columns, we get a buffer overflow. This always allocates a pool for the > columns. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> > > --- > .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsLib.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsLib.c > index 6ebf002..d81dd01 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsLib.c > +++ > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > dsLib.c > @@ -302,12 +302,21 @@ EditorClearLine ( > IN UINTN LastRow > ) > { > - CHAR16 Line[200]; > + CHAR16 *Line; > > if (Row == 0) { > Row = 1; > } > > + // Allocate buffer for columns > + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); > + if (Line == NULL) { > + return; > + } > + > // > // prepare a blank line > // > @@ -326,6 +335,10 @@ EditorClearLine ( > // print out the blank line > // > ShellPrintEx (0, ((INT32)Row) - 1, Line); > + > + FreePool (Line); > } > > /** > -- > 2.1.4 > > > -----Original Message----- > From: Carsey, Jaben [mailto:jaben.carsey@intel.com] > Sent: Dienstag, 24. Januar 2017 18:36 > To: Witt, Sebastian (DF FA AS DH KHE 1); edk2-devel@lists.01.org > Cc: Carsey, Jaben > Subject: RE: [PATCH] Fix edit on screens with more than 200 columns > > I didn't mean a 400 static, I meant start by allocating 400 and simplify the end > of the function... > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > > Witt, Sebastian > > Sent: Tuesday, January 24, 2017 8:48 AM > > To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org > > Subject: Re: [edk2] [PATCH] Fix edit on screens with more than 200 > > columns > > Importance: High > > > > Only performance. But I haven't measured if there is a big difference > > between static buffer and AllocateZeroPool. I wouldn't use a fixed value. > > There may be a display device with more than 400 columns. Otherwise > > one can always allocate the [LastCol + 1] buffer. > > > > -----Original Message----- > > From: Carsey, Jaben [mailto:jaben.carsey@intel.com] > > Sent: Dienstag, 24. Januar 2017 17:27 > > To: Witt, Sebastian (DF FA AS DH KHE 1); edk2-devel@lists.01.org > > Cc: Carsey, Jaben > > Subject: RE: [PATCH] Fix edit on screens with more than 200 columns > > > > Is there a reason to not just always start with allocating the 400 and > > then we don't need to complicate the end to conditionally free the buffer? > > > > > -----Original Message----- > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > > > Of Witt, Sebastian > > > Sent: Tuesday, January 24, 2017 5:14 AM > > > To: edk2-devel@lists.01.org > > > Subject: [edk2] [PATCH] Fix edit on screens with more than 200 > > > columns > > > Importance: High > > > > > > If the shell edit command is used on a screen with more than > > > 200 columns, we get a buffer overflow. This increases the default > > > buffer size to > > > 400 columns and allocates a pool when this is not enough. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Sebastian Witt <sebastian.witt@siemens.com> > > > > > > --- > > > .../UefiShellDebug1CommandsLib.c | 15 ++++++++++++++- > > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > > > diff --git > > > > > > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > > dsL > > > i > > > b.c > > > > > > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > > dsL > > > i > > > b.c > > > index 6ebf002..d81dd01 100644 > > > --- > > > > > > a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > > dsL > > > i > > > b.c > > > +++ > > > > > > b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman > > d > > > +++ sLib.c > > > @@ -302,12 +302,21 @@ EditorClearLine ( > > > IN UINTN LastRow > > > ) > > > { > > > - CHAR16 Line[200]; > > > + CHAR16 Buffer[400]; > > > + CHAR16 *Line = Buffer; > > > > > > if (Row == 0) { > > > Row = 1; > > > } > > > > > > + // If there are more columns than our buffer can contain, > > > + allocate new buffer if (LastCol >= (sizeof (Buffer) / sizeof (CHAR16))) { > > > + Line = AllocateZeroPool (LastCol*(sizeof(CHAR16) + 1)); > > > + if (Line == NULL) { > > > + return; > > > + } > > > + } > > > + > > > // > > > // prepare a blank line > > > // > > > @@ -326,6 +335,10 @@ EditorClearLine ( > > > // print out the blank line > > > // > > > ShellPrintEx (0, ((INT32)Row) - 1, Line); > > > + > > > + // Free if allocated > > > + if (Line != Buffer) > > > + FreePool (Line); > > > } > > > > > > /** > > > -- > > > 2.1.4 > > > > > > With best regards, > > > Sebastian Witt > > > > > > Siemens AG > > > Digital Factory Division > > > Factory Automation > > > Automation Products and Systems > > > DF FA AS DH KHE 1 > > > Oestliche Rheinbrueckenstr. 50 > > > 76187 Karlsruhe, Germany > > > Tel.: +49 721 595-5326 > > > mailto:sebastian.witt@siemens.com > > > > > > www.siemens.com/ingenuityforlife > > > > > > Siemens Aktiengesellschaft: Chairman of the Supervisory Board: > > > Gerhard Cromme; Managing Board: Joe Kaeser, Chairman, President and > > > Chief Executive Officer; Roland Busch, Lisa Davis, Klaus Helmrich, > > > Janina Kugel, Siegfried Russwurm, Ralf P. Thomas; Registered > > > offices: Berlin and Munich, Germany; Commercial registries: Berlin > > > Charlottenburg, HRB 12300, Munich, HRB 6684; WEEE-Reg.-No. DE > > > 23691322 _______________________________________________ > > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-26 17:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-24 13:13 [PATCH] Fix edit on screens with more than 200 columns Witt, Sebastian 2017-01-24 16:27 ` Carsey, Jaben 2017-01-24 16:47 ` Witt, Sebastian 2017-01-24 17:36 ` Carsey, Jaben 2017-01-26 9:00 ` Witt, Sebastian 2017-01-26 17:05 ` Carsey, Jaben
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox