<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=ZH-CN link=blue vlink="#954F72" style='word-wrap:break-word'><div class=WordSection1>Hi Zhichao,<o:p> </o:p> I will update the detailed description according to your suggestions，thanks.<o:p> </o:p>Chao.<o:p> </o:p>发送自 Windows 10 版<a href="https://go.microsoft.com/fwlink/?LinkId=550986">邮件</a>应用<o:p> </o:p><div style='mso-element:para-border-div;border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm'>发件人: <a href="mailto:zhichao.gao@intel.com">Gao, Zhichao</a> 发送时间: 2021年1月14日 10:06 收件人: <a href="mailto:gechao@greatwall.com.cn">gechao</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 fifobufferoverflow with UINT8 type</div><o:p> </o:p>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:0cm 0cm 0cm 4.0pt'><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm'>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 border=0 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 border=0 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 0cm 0cm 0cm'>发件人: <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></div><o:p> </o:p><o:p> </o:p></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/70322">View/Reply Online (#70322)</a> | | <a target="_blank" href="https://groups.io/mt/79682585/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>