public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer overflow.
@ 2021-01-14  3:22 gechao
  2021-01-14  6:34 ` Gao, Zhichao
       [not found] ` <165A05B49BF7F07B.12403@groups.io>
  0 siblings, 2 replies; 4+ messages in thread
From: gechao @ 2021-01-14  3:22 UTC (permalink / raw)
  To: zhichao.gao; +Cc: devel, ray.ni, gechao

From: gechao <gechao@greatwall.com.cn>

Fix the bug of terminal fifo buffer overflow with UINT8 type.

typedef struct {
  UINT8 Head;
  UINT8 Tail;
  UINT8 Data[RAW_FIFO_MAX_NUMBER + 1];
} RAW_DATA_FIFO;
RAW_FIFO_MAX_NUMBER is 256.
the data buffer size is 257 (Index from 0 to 256), but the max value of
the index, Head or Tail (UINT8), is 255. That means the last data of the
data buffer would be always empty if we use Head/Tail to output/input the
data correctly.  And because of the incorrect buffer size the FIFO full
check "((Tail + 1) % (RAW_FIFO_MAX_NUMBER + 1)) == Head" would never meet.

Signed-off-by: gechao <gechao@greatwall.com.cn>
---
 MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 378ace13ce..360e58e847 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -37,7 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/BaseLib.h>
 
 
-#define RAW_FIFO_MAX_NUMBER 256
+#define RAW_FIFO_MAX_NUMBER 255
 #define FIFO_MAX_NUMBER     128
 
 typedef struct {
-- 
2.25.1


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

* Re: [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer overflow.
  2021-01-14  3:22 [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer overflow gechao
@ 2021-01-14  6:34 ` Gao, Zhichao
       [not found] ` <165A05B49BF7F07B.12403@groups.io>
  1 sibling, 0 replies; 4+ messages in thread
From: Gao, Zhichao @ 2021-01-14  6:34 UTC (permalink / raw)
  To: gechao@greatwall.com.cn; +Cc: devel@edk2.groups.io, Ni, Ray

There is no need to add [BUG] in the title. I would remove it when I create the PR.
If you want to have a record for this bug, you can edit a Bugzilla and add the link in the commit message.
Beside of that, Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: gechao@greatwall.com.cn <gechao@greatwall.com.cn>
> Sent: Thursday, January 14, 2021 11:23 AM
> To: Gao, Zhichao <zhichao.gao@intel.com>
> Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; gechao
> <gechao@greatwall.com.cn>
> Subject: [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer
> overflow.
> 
> From: gechao <gechao@greatwall.com.cn>
> 
> Fix the bug of terminal fifo buffer overflow with UINT8 type.
> 
> typedef struct {
>   UINT8 Head;
>   UINT8 Tail;
>   UINT8 Data[RAW_FIFO_MAX_NUMBER + 1];
> } RAW_DATA_FIFO;
> RAW_FIFO_MAX_NUMBER is 256.
> the data buffer size is 257 (Index from 0 to 256), but the max value of the index,
> Head or Tail (UINT8), is 255. That means the last data of the data buffer would be
> always empty if we use Head/Tail to output/input the data correctly.  And
> because of the incorrect buffer size the FIFO full check "((Tail + 1) %
> (RAW_FIFO_MAX_NUMBER + 1)) == Head" would never meet.
> 
> Signed-off-by: gechao <gechao@greatwall.com.cn>
> ---
>  MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 378ace13ce..360e58e847 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -37,7 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/BaseLib.h>  -#define RAW_FIFO_MAX_NUMBER 256+#define
> RAW_FIFO_MAX_NUMBER 255 #define FIFO_MAX_NUMBER     128  typedef
> struct {--
> 2.25.1


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

* Re: [edk2-devel] [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer overflow.
       [not found] ` <165A05B49BF7F07B.12403@groups.io>
@ 2021-01-15  8:49   ` Gao, Zhichao
  2021-01-15  8:53     ` Wu, Hao A
  0 siblings, 1 reply; 4+ messages in thread
From: Gao, Zhichao @ 2021-01-15  8:49 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Zhichao, gechao@greatwall.com.cn,
	Wu, Hao A, Wang, Jian J
  Cc: Ni, Ray

Add Hao and Jian, who have the permission to merge the patch. CI result: https://github.com/tianocore/edk2/pull/1358

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao, Zhichao
> Sent: Thursday, January 14, 2021 2:35 PM
> To: gechao@greatwall.com.cn
> Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal
> fifo buffer overflow.
> 
> There is no need to add [BUG] in the title. I would remove it when I create the PR.
> If you want to have a record for this bug, you can edit a Bugzilla and add the link
> in the commit message.
> Beside of that, Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: gechao@greatwall.com.cn <gechao@greatwall.com.cn>
> > Sent: Thursday, January 14, 2021 11:23 AM
> > To: Gao, Zhichao <zhichao.gao@intel.com>
> > Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; gechao
> > <gechao@greatwall.com.cn>
> > Subject: [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer
> > overflow.
> >
> > From: gechao <gechao@greatwall.com.cn>
> >
> > Fix the bug of terminal fifo buffer overflow with UINT8 type.
> >
> > typedef struct {
> >   UINT8 Head;
> >   UINT8 Tail;
> >   UINT8 Data[RAW_FIFO_MAX_NUMBER + 1]; } RAW_DATA_FIFO;
> > RAW_FIFO_MAX_NUMBER is 256.
> > the data buffer size is 257 (Index from 0 to 256), but the max value
> > of the index, Head or Tail (UINT8), is 255. That means the last data
> > of the data buffer would be always empty if we use Head/Tail to
> > output/input the data correctly.  And because of the incorrect buffer
> > size the FIFO full check "((Tail + 1) % (RAW_FIFO_MAX_NUMBER + 1)) == Head"
> would never meet.
> >
> > Signed-off-by: gechao <gechao@greatwall.com.cn>
> > ---
> >  MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > index 378ace13ce..360e58e847 100644
> > --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > @@ -37,7 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include <Library/BaseLib.h>  -#define RAW_FIFO_MAX_NUMBER 256+#define
> > RAW_FIFO_MAX_NUMBER 255 #define FIFO_MAX_NUMBER     128  typedef
> > struct {--
> > 2.25.1
> 
> 
> 
> 
> 


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

* Re: [edk2-devel] [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer overflow.
  2021-01-15  8:49   ` [edk2-devel] " Gao, Zhichao
@ 2021-01-15  8:53     ` Wu, Hao A
  0 siblings, 0 replies; 4+ messages in thread
From: Wu, Hao A @ 2021-01-15  8:53 UTC (permalink / raw)
  To: Gao, Zhichao, devel@edk2.groups.io, gechao@greatwall.com.cn,
	Wang, Jian J
  Cc: Ni, Ray

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Friday, January 15, 2021 4:49 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>;
> gechao@greatwall.com.cn; Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/TerminalDxe [BUG]:
> Terminal fifo buffer overflow.
> 
> Add Hao and Jian, who have the permission to merge the patch. CI result:
> https://github.com/tianocore/edk2/pull/1358


Hello Zhichao,

I have added the 'push' label for the PR.

Best Regards,
Hao Wu


> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao,
> > Zhichao
> > Sent: Thursday, January 14, 2021 2:35 PM
> > To: gechao@greatwall.com.cn
> > Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/TerminalDxe [BUG]:
> > Terminal fifo buffer overflow.
> >
> > There is no need to add [BUG] in the title. I would remove it when I create
> the PR.
> > If you want to have a record for this bug, you can edit a Bugzilla and
> > add the link in the commit message.
> > Beside of that, Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
> >
> > Thanks,
> > Zhichao
> >
> > > -----Original Message-----
> > > From: gechao@greatwall.com.cn <gechao@greatwall.com.cn>
> > > Sent: Thursday, January 14, 2021 11:23 AM
> > > To: Gao, Zhichao <zhichao.gao@intel.com>
> > > Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; gechao
> > > <gechao@greatwall.com.cn>
> > > Subject: [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo
> > > buffer overflow.
> > >
> > > From: gechao <gechao@greatwall.com.cn>
> > >
> > > Fix the bug of terminal fifo buffer overflow with UINT8 type.
> > >
> > > typedef struct {
> > >   UINT8 Head;
> > >   UINT8 Tail;
> > >   UINT8 Data[RAW_FIFO_MAX_NUMBER + 1]; } RAW_DATA_FIFO;
> > > RAW_FIFO_MAX_NUMBER is 256.
> > > the data buffer size is 257 (Index from 0 to 256), but the max value
> > > of the index, Head or Tail (UINT8), is 255. That means the last data
> > > of the data buffer would be always empty if we use Head/Tail to
> > > output/input the data correctly.  And because of the incorrect
> > > buffer size the FIFO full check "((Tail + 1) % (RAW_FIFO_MAX_NUMBER +
> 1)) == Head"
> > would never meet.
> > >
> > > Signed-off-by: gechao <gechao@greatwall.com.cn>
> > > ---
> > >  MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > > b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > > index 378ace13ce..360e58e847 100644
> > > --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > > @@ -37,7 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > #include <Library/BaseLib.h>  -#define RAW_FIFO_MAX_NUMBER
> 256+#define
> > > RAW_FIFO_MAX_NUMBER 255 #define FIFO_MAX_NUMBER     128
> typedef
> > > struct {--
> > > 2.25.1
> >
> >
> >
> > 
> >


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

end of thread, other threads:[~2021-01-15  8:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-14  3:22 [PATCH] MdeModulePkg/TerminalDxe [BUG]: Terminal fifo buffer overflow gechao
2021-01-14  6:34 ` Gao, Zhichao
     [not found] ` <165A05B49BF7F07B.12403@groups.io>
2021-01-15  8:49   ` [edk2-devel] " Gao, Zhichao
2021-01-15  8:53     ` Wu, Hao A

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