From: "Nickle Wang" <nickle.wang@hpe.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"gaoliming@byosoft.com.cn" <gaoliming@byosoft.com.cn>,
"lersek@redhat.com" <lersek@redhat.com>
Cc: "jian.j.wang@intel.com" <jian.j.wang@intel.com>,
"hao.a.wu@intel.com" <hao.a.wu@intel.com>,
"Wang, Nickle (HPS SW)" <nickle.wang@hpe.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/RegularExpressionDxe: Fix memory assert in FreePool()
Date: Mon, 5 Jul 2021 00:55:17 +0000 [thread overview]
Message-ID: <DF4PR8401MB0812D0D9C390357709189A15FF1C9@DF4PR8401MB0812.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <011001d76e38$97615000$c623f000$@byosoft.com.cn>
Thanks Liming! I create personal PR below and I also see CI error on OnigurumaUefiPort.h. It looks like "Ubuntu GCC PR" does not like the coding style on OnigurumaUefiPort.h. However, I just follow the coding style of other functions in this file.
https://github.com/tianocore/edk2/pull/1788
So, should I add this file into exception list too? Or I just follow the coding style on what I modified?
Thanks,
Nickle
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Thursday, July 1, 2021 1:19 PM
To: devel@edk2.groups.io; Wang, Nickle (HPS SW) <nickle.wang@hpe.com>; lersek@redhat.com
Cc: jian.j.wang@intel.com; hao.a.wu@intel.com
Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/RegularExpressionDxe: Fix memory assert in FreePool()
Nickle:
You can create personal PR to verify this change first. If it passes CI, please send the updated patch set.
Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Nickle Wang
> 发送时间: 2021年6月30日 21:11
> 收件人: devel@edk2.groups.io; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>; gaoliming <gaoliming@byosoft.com.cn>;
> lersek@redhat.com
> 抄送: jian.j.wang@intel.com; hao.a.wu@intel.com; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>
> 主题: Re: [edk2-devel] [PATCH] MdeModulePkg/RegularExpressionDxe: Fix
> memory assert in FreePool()
>
> Hi Liming,
>
> I got my patch ready. Should I test it by creating PR on Github like
> https://github.com/tianocore/edk2/pull/1735? Or I just send out new
> patch for review?
>
> Thanks,
> Nickle
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle
> Wang
> Sent: Wednesday, June 30, 2021 9:49 AM
> To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> lersek@redhat.com
> Cc: jian.j.wang@intel.com; hao.a.wu@intel.com; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/RegularExpressionDxe:
> Fix memory assert in FreePool()
>
> I see. Thanks for your quick response. I will work on it.
>
> Nickle
>
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Wednesday, June 30, 2021 9:43 AM
> To: Wang, Nickle (HPS SW) <nickle.wang@hpe.com>; devel@edk2.groups.io;
> lersek@redhat.com
> Cc: jian.j.wang@intel.com; hao.a.wu@intel.com
> Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/RegularExpressionDxe:
> Fix memory assert in FreePool()
>
> This is a separate commit. You can send it together with previous patch.
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Wang, Nickle (HPS SW) <nickle.wang@hpe.com>
> > 发送时间: 2021年6月30日 9:29
> > 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> > lersek@redhat.com
> > 抄送: jian.j.wang@intel.com; hao.a.wu@intel.com; Wang, Nickle (HPS SW)
> > <nickle.wang@hpe.com>
> > 主题: RE: [edk2-devel] [PATCH] MdeModulePkg/RegularExpressionDxe: Fix
> > memory assert in FreePool()
> >
> > Hi Liming,
> >
> > No problem. It looks like I have to add OnigurumaUefiPort.c into
> "IgnoreFiles"
> > object in MdeModulePkg.ci.yaml. And should I send this patch alone?
> > Or I have to send it as a part of patch in early fix?
> >
> > Thanks,
> > Nickle
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Wednesday, June 30, 2021 9:06 AM
> > To: devel@edk2.groups.io; lersek@redhat.com; Wang, Nickle (HPS SW)
> > <nickle.wang@hpe.com>
> > Cc: jian.j.wang@intel.com; hao.a.wu@intel.com
> > Subject: 回复: [edk2-devel] [PATCH]
> MdeModulePkg/RegularExpressionDxe:
> > Fix memory assert in FreePool()
> >
> > Laszlo:
> > Yes. I agree to add OnigurumaUefiPort.c into ECC exception in
> > MdeModulePkg.ci.yaml.
> >
> > Nickle:
> > Can you provide the patch to update MdeModulePkg.ci.yaml?
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Laszlo
> > Ersek
> > > 发送时间: 2021年6月29日 22:52
> > > 收件人: devel@edk2.groups.io; nickle.wang@hpe.com
> > > 抄送: gaoliming@byosoft.com.cn; jian.j.wang@intel.com;
> > > hao.a.wu@intel.com
> > > 主题: Re: [edk2-devel] [PATCH] MdeModulePkg/RegularExpressionDxe:
> Fix
> > > memory assert in FreePool()
> > >
> > > On 06/10/21 06:56, Nickle Wang wrote:
> > > > Memory buffer that is allocated by malloc() and realloc() will
> > > > be shifted by 8 bytes because Oniguruma keeps its memory signature.
> > > > This 8 bytes shift is not handled while calling free() to
> > > > release memory. Add
> > > > free() function to check Oniguruma signature before release
> > > > memory because memory buffer is not touched when using calloc().
> > > >
> > > > Signed-off-by: Nickle Wang <nickle.wang@hpe.com>
> > > > ---
> > > > .../RegularExpressionDxe/OnigurumaUefiPort.c | 19
> > > ++++++++++++++++++-
> > > > .../RegularExpressionDxe/OnigurumaUefiPort.h | 14
> > > > ++------------
> > > > 2 files changed, 20 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git
> > > a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
> > > b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
> > > > index 9aa7b0a68e..5c34324db8 100644
> > > > ---
> > > a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
> > > > +++
> > > b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
> > > > @@ -2,7 +2,7 @@
> > > >
> > > > Module to rewrite stdlib references within Oniguruma
> > > >
> > > > - (C) Copyright 2014-2015 Hewlett Packard Enterprise
> > > > Development
> > > LP<BR>
> > > > + (C) Copyright 2014-2021 Hewlett Packard Enterprise
> > > > + Development
> > > LP<BR>
> > > > Copyright (c) 2020, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -96,3 +96,20
> @@
> > > > void* memset (void *dest, char ch, unsigned int
> > > count)
> > > > return SetMem (dest, count, ch); }
> > > >
> > > > +void free(void *ptr)
> > > > +{
> > > > + VOID *EvalOnce;
> > > > + ONIGMEM_HEAD *PoolHdr;
> > > > +
> > > > + EvalOnce = ptr;
> > > > + if (EvalOnce == NULL) {
> > > > + return;
> > > > + }
> > > > +
> > > > + PoolHdr = (ONIGMEM_HEAD *)EvalOnce - 1;
> > > > + if (PoolHdr->Signature == ONIGMEM_HEAD_SIGNATURE) {
> > > > + FreePool (PoolHdr);
> > > > + } else {
> > > > + FreePool (EvalOnce);
> > > > + }
> > > > +}
> > > > diff --git
> > > a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
> > > b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
> > > > index 20b75c3361..0bdb7be529 100644
> > > > ---
> > > a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
> > > > +++
> > > b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
> > > > @@ -2,7 +2,7 @@
> > > >
> > > > Module to rewrite stdlib references within Oniguruma
> > > >
> > > > - (C) Copyright 2014-2015 Hewlett Packard Enterprise
> > > > Development
> > > LP<BR>
> > > > + (C) Copyright 2014-2021 Hewlett Packard Enterprise
> > > > + Development
> > > LP<BR>
> > > > Copyright (c) 2020, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > @@ -46,17 +46,6 @@ typedef INTN intptr_t;
> > > > #endif
> > > >
> > > > #define calloc(n,s) AllocateZeroPool((n)*(s))
> > > > -
> > > > -#define free(p) \
> > > > - do { \
> > > > - VOID *EvalOnce; \
> > > > - \
> > > > - EvalOnce = (p); \
> > > > - if (EvalOnce != NULL) { \
> > > > - FreePool (EvalOnce); \
> > > > - } \
> > > > - } while (FALSE)
> > > > -
> > > > #define xmemmove(Dest,Src,Length) CopyMem(Dest,Src,Length)
> > #define
> > > > xmemcpy(Dest,Src,Length) CopyMem(Dest,Src,Length) #define
> > > > xmemset(Buffer,Value,Length) SetMem(Buffer,Length,Value) @@
> > > > -98,6
> > > > +87,7 @@ void* malloc(size_t size);
> > > > void* realloc(void *ptr, size_t size);
> > > > void* memcpy (void *dest, const void *src, unsigned int count);
> > > > void* memset (void *dest, char ch, unsigned int count);
> > > > +void free(void *ptr);
> > > >
> > > > #define exit(n) ASSERT(FALSE);
> > > >
> > > >
> > >
> > > This patch cannot be merged, due to a number of EccCheck complaints:
> > >
> > > 2021-06-21T01:44:13.4327861Z PROGRESS - --Running MdeModulePkg:
> > > EccCheck Test NO-TARGET --
> > > 2021-06-21T01:44:20.4922300Z ERROR - 2021-06-21T01:44:20.4924178Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.4925524Z ERROR
> > > - *Error code: 4002 2021-06-21T01:44:20.4927323Z ERROR - *Function
> > > header doesn't exist 2021-06-21T01:44:20.4936437Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.c
> > > 2021-06-21T01:44:20.4937669Z ERROR - *Line number: 99
> > > 2021-06-21T01:44:20.4938737Z ERROR - *Function [free] has NO
> comment
> > > immediately preceding it.
> > > 2021-06-21T01:44:20.4945489Z ERROR - 2021-06-21T01:44:20.4951382Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.4960149Z ERROR
> > > - *Error code: 4002 2021-06-21T01:44:20.4961161Z ERROR - *Function
> > > header doesn't exist 2021-06-21T01:44:20.4966674Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.4973232Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.4978337Z ERROR - *Function [free] has NO
> comment
> > > immediately preceding it.
> > > 2021-06-21T01:44:20.4981257Z ERROR - 2021-06-21T01:44:20.4983805Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.4986537Z ERROR
> > > - *Error code: 5001 2021-06-21T01:44:20.4989508Z ERROR - *Return
> > > type of a function should exist and in the first line
> > > 2021-06-21T01:44:20.4997043Z ERROR -
> > > *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.4997804Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.4998331Z ERROR - *[free] Return Type should
> > > appear on its own line 2021-06-21T01:44:20.4998762Z ERROR -
> > > 2021-06-21T01:44:20.4999175Z ERROR - EFI coding style error
> > > 2021-06-21T01:44:20.5017351Z ERROR - *Error code: 5003
> > > 2021-06-21T01:44:20.5023282Z ERROR - *Function name should be left
> > > justified, followed by the beginning of the parameter list, with
> > > the closing parenthesis on its own line, indented two spaces
> > > 2021-06-21T01:44:20.5024931Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.c
> > > 2021-06-21T01:44:20.5025818Z ERROR - *Line number: 99
> > > 2021-06-21T01:44:20.5026960Z ERROR - *Function name [free] should
> > > appear at the start of a line 2021-06-21T01:44:20.5027533Z ERROR -
> > > 2021-06-21T01:44:20.5027982Z ERROR - EFI coding style error
> > > 2021-06-21T01:44:20.5028454Z ERROR - *Error code: 5003
> > > 2021-06-21T01:44:20.5029279Z ERROR - *Function name should be left
> > > justified, followed by the beginning of the parameter list, with
> > > the closing parenthesis on its own line, indented two spaces
> > > 2021-06-21T01:44:20.5030177Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.5030770Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.5031330Z ERROR - *Function name [free] should
> > > appear at the start of a line 2021-06-21T01:44:20.5031788Z ERROR -
> > > 2021-06-21T01:44:20.5032240Z ERROR - EFI coding style error
> > > 2021-06-21T01:44:20.5032706Z ERROR - *Error code: 5003
> > > 2021-06-21T01:44:20.5033554Z ERROR - *Function name should be left
> > > justified, followed by the beginning of the parameter list, with
> > > the closing parenthesis on its own line, indented two spaces
> > > 2021-06-21T01:44:20.5036470Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.5040063Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.5043513Z ERROR - *Parameter ptr should be in
> > > its own line.
> > > 2021-06-21T01:44:20.5046782Z ERROR - 2021-06-21T01:44:20.5049909Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.5053571Z ERROR
> > > - *Error code: 5003 2021-06-21T01:44:20.5057415Z ERROR - *Function
> > > name should be left justified, followed by the beginning of the
> > > parameter list, with the closing parenthesis on its own line,
> > > indented two spaces 2021-06-21T01:44:20.5066200Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.5066831Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.5067378Z ERROR - *')' should be on a new line
> > > and indented two spaces 2021-06-21T01:44:20.5067799Z ERROR -
> > > 2021-06-21T01:44:20.5068211Z ERROR - EFI coding style error
> > > 2021-06-21T01:44:20.5070600Z ERROR - *Error code: 7001
> > > 2021-06-21T01:44:20.5074448Z ERROR - *There should be no use of
> > > int, unsigned, char, void, long in any .c, .h or .asl files
> > > 2021-06-21T01:44:20.5077965Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.c
> > > 2021-06-21T01:44:20.5081222Z ERROR - *Line number: 110
> > > 2021-06-21T01:44:20.5084248Z ERROR - *Parameter ptr
> > > 2021-06-21T01:44:20.5090115Z ERROR - 2021-06-21T01:44:20.5090517Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.5090923Z ERROR
> > > - *Error code: 7001 2021-06-21T01:44:20.5093481Z ERROR - *There
> should
> > > be no use of int, unsigned, char, void, long in any .c, .h or .asl
> > > files 2021-06-21T01:44:20.5096387Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.5099658Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.5103008Z ERROR - *free Return type void
> > > 2021-06-21T01:44:20.5105878Z ERROR - 2021-06-21T01:44:20.5108537Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.5111630Z ERROR
> > > - *Error code: 7001 2021-06-21T01:44:20.5115083Z ERROR - *There
> should
> > > be no use of int, unsigned, char, void, long in any .c, .h or .asl
> > > files 2021-06-21T01:44:20.5118600Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.5126189Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.5142100Z ERROR - *Parameter ptr
> > > 2021-06-21T01:44:20.5142574Z ERROR - 2021-06-21T01:44:20.5142979Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.5143429Z ERROR
> > > - *Error code: 8005 2021-06-21T01:44:20.5144332Z ERROR - *Variable
> > > name does not follow the
> > > rules: 1. First character should be upper case 2. Must contain
> > > lower case characters 3. No white space characters 4. Global
> > > variable name must start with a 'g'
> > > 2021-06-21T01:44:20.5145416Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.h
> > > 2021-06-21T01:44:20.5146050Z ERROR - *Line number: 90
> > > 2021-06-21T01:44:20.5146555Z ERROR - *Parameter [ptr] NOT follow
> > > naming convention.
> > > 2021-06-21T01:44:20.5146963Z ERROR - 2021-06-21T01:44:20.5147366Z
> > > ERROR - EFI coding style error 2021-06-21T01:44:20.5147794Z ERROR
> > > - *Error code: 8006 2021-06-21T01:44:20.5148562Z ERROR - *Function
> > > name does not follow the
> > > rules: 1. First character should be upper case 2. Must contain
> > > lower case characters 3. No white space characters
> > > 2021-06-21T01:44:20.5149399Z ERROR - *file:
> > >
> >
> //home/vsts/work/1/s/MdeModulePkg/Universal/RegularExpressionDxe/Oni
> > > gurumaUefiPort.c
> > > 2021-06-21T01:44:20.5149932Z ERROR - *Line number: 99
> > > 2021-06-21T01:44:20.5150445Z ERROR - *The function name [free]
> > > does not follow the rules 2021-06-21T01:44:20.5155470Z ERROR -
> > > --->Test
> > > Failed: EccCheck Test NO-TARGET returned 1
> > >
> > > These are all (or mostly) related to preexistent code, so I think
> > > the EccCheck plugin config should be updated in MdeModulePkg, to
> > > permit an exception for these files.
> > >
> > > For now, I've closed <https://github.com/tianocore/edk2/pull/1735>
> > > without merging it.
> > >
> > > Thanks
> > > Laszlo
> > >
> > >
> > >
> > >
> > >
> >
> >
>
>
>
>
>
>
>
>
>
>
>
>
next prev parent reply other threads:[~2021-07-05 1:12 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 4:56 [PATCH] MdeModulePkg/RegularExpressionDxe: Fix memory assert in FreePool() Nickle Wang
2021-06-11 2:24 ` 回复: " gaoliming
2021-06-29 14:51 ` [edk2-devel] " Laszlo Ersek
2021-06-30 1:06 ` 回复: " gaoliming
2021-06-30 1:28 ` Nickle Wang
2021-06-30 1:42 ` 回复: " gaoliming
2021-06-30 1:48 ` Nickle Wang
[not found] ` <168D3906BAF6D783.28904@groups.io>
2021-06-30 13:11 ` Nickle Wang
2021-07-01 5:18 ` 回复: " gaoliming
2021-07-05 0:55 ` Nickle Wang [this message]
2021-07-05 1:46 ` gaoliming
2021-07-05 2:44 ` Nickle Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DF4PR8401MB0812D0D9C390357709189A15FF1C9@DF4PR8401MB0812.NAMPRD84.PROD.OUTLOOK.COM \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox