<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="#954F72"> <div class="WordSection1"> Hi,<o:p></o:p> <o:p> </o:p> Now I understood. It is a bug fix. But the commit message is not clear.<o:p></o:p> <o:p> </o:p> typedef struct {<o:p></o:p> UINT8 Head;<o:p></o:p> UINT8 Tail;<o:p></o:p> UINT8 Data[RAW_FIFO_MAX_NUMBER + 1];<o:p></o:p> } RAW_DATA_FIFO;<o:p></o:p> RAW_FIFO_MAX_NUMBER is 256.<o:p></o:p> 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.<o:p></o:p> And because of the incorrect buffer size the FIFO full check “((Tail + 1) % (RAW_FIFO_MAX_NUMBER + 1)) == Head” would never meet.<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Zhichao<o:p></o:p> <o:p> </o:p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: gechao <gechao@greatwall.com.cn> Sent: Wednesday, January 13, 2021 5:12 PM To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io Cc: Ni, Ray <ray.ni@intel.com> Subject: 回复: [PATCH] MdeModulePkg/TerminalDxe: Fix terminal fifo bufferoverflow with UINT8 type<o:p></o:p> </div> </div> <o:p> </o:p> Hi Zhigao,<o:p></o:p> <o:p> </o:p> Let's take the following code as an example, as we know, The maximum data represented by the UINT8 type is 255, <o:p></o:p> So the equation condition on line 812 will not be true under special circumstances in picture 1, because (RAW_FIFO_MAX_NUMBER + 1) = 257,<o:p></o:p> So when the fifo buffer is full with head = 0 and Tail = 255, the maximum value of (Tail + 1) % (RAW_FIFO_MAX_NUMBER + 1) is 256,<o:p></o:p> This function will return false, This situation will occur in rare cases.<o:p></o:p> <o:p> </o:p> This is a classic case we encountered in the project, our program will hang in picture 2 in this situation when the serial port does not respond.<o:p></o:p> <o:p> </o:p> <o:p> </o:p> File: MdeModulePkg\Universal\Console\TerminalDxe\TerminalConIn.c<o:p></o:p> <o:p> </o:p> picture 1:<o:p></o:p> <o:p> </o:p> <img width="515" height="314" style="width:5.3645in;height:3.2708in" id="图片_x0020_3" src="cid:image001.png@01D6EA58.3B80D990"><o:p></o:p> <o:p> </o:p> picture 2:<o:p></o:p> <img width="512" height="322" style="width:5.3333in;height:3.3541in" id="图片_x0020_4" src="cid:image002.png@01D6EA58.3B80D990"><o:p></o:p> <o:p> </o:p> <o:p> </o:p> 发送自 Windows 10 版<a href="https://go.microsoft.com/fwlink/?LinkId=550986">邮件</a>应用<o:p></o:p> <o:p> </o:p> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> 发件人: <a href="mailto:zhichao.gao@intel.com">Gao, Zhichao</a> 发送时间: 2021年1月13日 14:57 收件人: <a href="mailto:gechao@greatwall.com.cn">gechao@greatwall.com.cn</a>; <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> 抄送: <a href="mailto:ray.ni@intel.com">Ni, Ray</a> 主题: RE: [PATCH] MdeModulePkg/TerminalDxe: Fix terminal fifo bufferoverflow with UINT8 type<o:p></o:p> </div> <o:p> </o:p> Hi,<o:p></o:p> <o:p> </o:p> Sorry, I don't understand the patch. UINT8 type would have the value limitation. But why does it affect the buffer size?<o:p></o:p> Did you observed the overflow with the original value? If yes, can you share the example?<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Zhichao<o:p></o:p> <o:p> </o:p> > -----Original Message-----<o:p></o:p> > From: <a href="mailto:gechao@greatwall.com.cn">gechao@greatwall.com.cn</a> <<a href="mailto:gechao@greatwall.com.cn">gechao@greatwall.com.cn</a>><o:p></o:p> > Sent: Tuesday, December 22, 2020 6:19 PM<o:p></o:p> > To: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>; Gao, Zhichao <<a href="mailto:zhichao.gao@intel.com">zhichao.gao@intel.com</a>><o:p></o:p> > Cc: Ni, Ray <<a href="mailto:ray.ni@intel.com">ray.ni@intel.com</a>>; gechao <<a href="mailto:gechao@greatwall.com.cn">gechao@greatwall.com.cn</a>><o:p></o:p> > Subject: [PATCH] MdeModulePkg/TerminalDxe: Fix terminal fifo buffer<o:p></o:p> > overflow with UINT8 type<o:p></o:p> > <o:p></o:p> > From: gechao <<a href="mailto:gechao@greatwall.com.cn">gechao@greatwall.com.cn</a>><o:p></o:p> > <o:p></o:p> > The maximum fifo buffer length is RAW_FIFO_MAX_NUMBER + 1 = 257, but<o:p></o:p> > the maximum value of terminal fifo buffer index is sizeof(UINT8) - 1 = 255 with<o:p></o:p> > UINT8 type, so check if fifo buffer is empty or full with below expression, ((Tail<o:p></o:p> > + 1) % (RAW_FIFO_MAX_NUMBER + 1)) == Head, (Tail + 1) might be<o:p></o:p> > sizeof(UINT8) + 1 = 256, for UINT8 type, it does not make any sense.<o:p></o:p> > <o:p></o:p> > Signed-off-by: gechao <<a href="mailto:gechao@greatwall.com.cn">gechao@greatwall.com.cn</a>><o:p></o:p> > ---<o:p></o:p> > MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 2 +-<o:p></o:p> > 1 file changed, 1 insertion(+), 1 deletion(-)<o:p></o:p> > <o:p></o:p> > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h<o:p></o:p> > b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h<o:p></o:p> > index 378ace13ce..360e58e847 100644<o:p></o:p> > --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h<o:p></o:p> > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h<o:p></o:p> > @@ -37,7 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent<o:p></o:p> > #include <Library/BaseLib.h> -#define RAW_FIFO_MAX_NUMBER<o:p></o:p> > 256+#define RAW_FIFO_MAX_NUMBER 255 #define FIFO_MAX_NUMBER<o:p></o:p> > 128 typedef struct {--<o:p></o:p> > 2.25.1<o:p></o:p> <o:p> </o:p> </div> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/70267">View/Reply Online (#70267)</a> | | <a target="_blank" href="https://groups.io/mt/79667138/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com] <div width="1" style="color:white;clear:both">_._,_._,_</div>