* [PATCH] MdeModulePkg/Variable: add debug logs in VariableServiceSetVariable @ 2018-11-20 10:38 Vijayenthiran Subramaniam 2018-11-20 11:17 ` Laszlo Ersek 0 siblings, 1 reply; 3+ messages in thread From: Vijayenthiran Subramaniam @ 2018-11-20 10:38 UTC (permalink / raw) To: edk2-devel, star.zeng, jian.j.wang, ruiyu.ni; +Cc: Vijayenthiran Subramaniam Print debug messages if size of the VariableName plus DataSize exceeds Max(Auth|Voltaile)VariableSize bytes. The messages will be useful if any platform specific value of Max(Auth|Voltaile)VariableSize PCDs have to be changed. Cc: Star Zeng <star.zeng@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com> --- MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c index 8e8db71bd201..db54fa4412c0 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c @@ -3234,14 +3234,20 @@ VariableServiceSetVariable ( // if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) { if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) { + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxAuthVariableSize.\n", + __FUNCTION__, VariableName)); return EFI_INVALID_PARAMETER; } } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) { if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) { + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxVariableSize.\n", + __FUNCTION__, VariableName)); return EFI_INVALID_PARAMETER; } } else { if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ()) { + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxVolatileVariableSize.\n", + __FUNCTION__, VariableName)); return EFI_INVALID_PARAMETER; } } -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] MdeModulePkg/Variable: add debug logs in VariableServiceSetVariable 2018-11-20 10:38 [PATCH] MdeModulePkg/Variable: add debug logs in VariableServiceSetVariable Vijayenthiran Subramaniam @ 2018-11-20 11:17 ` Laszlo Ersek 2018-11-21 1:47 ` Zeng, Star 0 siblings, 1 reply; 3+ messages in thread From: Laszlo Ersek @ 2018-11-20 11:17 UTC (permalink / raw) To: Vijayenthiran Subramaniam, edk2-devel, star.zeng, jian.j.wang, ruiyu.ni On 11/20/18 11:38, Vijayenthiran Subramaniam wrote: > Print debug messages if size of the VariableName plus DataSize exceeds > Max(Auth|Voltaile)VariableSize bytes. The messages will be useful if any > platform specific value of Max(Auth|Voltaile)VariableSize PCDs have to > be changed. > > Cc: Star Zeng <star.zeng@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Ruiyu Ni <ruiyu.ni@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com> > --- > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > index 8e8db71bd201..db54fa4412c0 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -3234,14 +3234,20 @@ VariableServiceSetVariable ( > // > if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) { > if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) { > + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxAuthVariableSize.\n", > + __FUNCTION__, VariableName)); > return EFI_INVALID_PARAMETER; > } > } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) { > if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) { > + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxVariableSize.\n", > + __FUNCTION__, VariableName)); > return EFI_INVALID_PARAMETER; > } > } else { > if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ()) { > + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxVolatileVariableSize.\n", > + __FUNCTION__, VariableName)); > return EFI_INVALID_PARAMETER; > } > } > You could make this more useful as well -- again, *if* the package maintainers agree with the new log messages --; namely, the vendor GUID, DataSize, and the limit in question should / could all be logged. Thanks Laszlo ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] MdeModulePkg/Variable: add debug logs in VariableServiceSetVariable 2018-11-20 11:17 ` Laszlo Ersek @ 2018-11-21 1:47 ` Zeng, Star 0 siblings, 0 replies; 3+ messages in thread From: Zeng, Star @ 2018-11-21 1:47 UTC (permalink / raw) To: Laszlo Ersek, Vijayenthiran Subramaniam, edk2-devel, jian.j.wang, ruiyu.ni Cc: star.zeng On 2018/11/20 19:17, Laszlo Ersek wrote: > On 11/20/18 11:38, Vijayenthiran Subramaniam wrote: >> Print debug messages if size of the VariableName plus DataSize exceeds >> Max(Auth|Voltaile)VariableSize bytes. The messages will be useful if any >> platform specific value of Max(Auth|Voltaile)VariableSize PCDs have to >> be changed. >> >> Cc: Star Zeng <star.zeng@intel.com> >> Cc: Jian J Wang <jian.j.wang@intel.com> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com> >> --- >> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> index 8e8db71bd201..db54fa4412c0 100644 >> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> @@ -3234,14 +3234,20 @@ VariableServiceSetVariable ( >> // >> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) { >> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) { >> + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxAuthVariableSize.\n", >> + __FUNCTION__, VariableName)); >> return EFI_INVALID_PARAMETER; >> } >> } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) { >> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) { >> + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxVariableSize.\n", >> + __FUNCTION__, VariableName)); >> return EFI_INVALID_PARAMETER; >> } >> } else { >> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ()) { >> + DEBUG ((DEBUG_ERROR, "%a: Size of (%s) variable + DataSize exceeds MaxVolatileVariableSize.\n", >> + __FUNCTION__, VariableName)); >> return EFI_INVALID_PARAMETER; >> } >> } >> > > You could make this more useful as well -- again, *if* the package > maintainers agree with the new log messages --; namely, the vendor GUID, > DataSize, and the limit in question should / could all be logged. I agree to add these debug messages. I also think Laszlo's suggestion is good. So, how about having the debug message like below? DEBUG (( DEBUG_ERROR, "%a: %s:%g\n", __FUNCTION__, VariableName, VendorGuid )); DEBUG (( DEBUG_ERROR, " NameSize(0x%x) + PayloadSize(0x%x) > MaxAuthVariableSize(0x%x) - HeaderSize(0x%x)\n", StrSize (VariableName), PayloadSize, mVariableModuleGlobal->MaxAuthVariableSize, GetVariableHeaderSize () )); Thanks, Star > > Thanks > Laszlo > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-21 1:47 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-20 10:38 [PATCH] MdeModulePkg/Variable: add debug logs in VariableServiceSetVariable Vijayenthiran Subramaniam 2018-11-20 11:17 ` Laszlo Ersek 2018-11-21 1:47 ` Zeng, Star
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox