* [PATCH] MdeModulePkg VariableSmm: Check InfoSize correctly
@ 2016-12-09 8:47 Star Zeng
2016-12-11 13:28 ` Yao, Jiewen
0 siblings, 1 reply; 2+ messages in thread
From: Star Zeng @ 2016-12-09 8:47 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Jiewen Yao
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=290
Current SmmVariableGetStatistics() in VariableSmm.c is always
checking input InfoSize against the first variable info,
it is incorrect.
For instance, there are three variables.
BootOrder
Boot0000
Boot0001
If the input InfoEntry is holding the second variable info (Boot0000)
and InfoSize is sizeof (VARIABLE_INFO_ENTRY) + StrSize (L"Boot0000"),
current code will return EFI_BUFFER_TOO_SMALL, but it should return
the third variable info (Boot0001).
This patch is to refine the code logic.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
.../Universal/Variable/RuntimeDxe/VariableSmm.c | 25 +++++++++++++++-------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index eafb53322e8c..85158d8b46ae 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -349,9 +349,10 @@ SmmVariableGetStatistics (
)
{
VARIABLE_INFO_ENTRY *VariableInfo;
- UINTN NameLength;
+ UINTN NameSize;
UINTN StatisticsInfoSize;
CHAR16 *InfoName;
+ UINTN InfoNameMaxSize;
EFI_GUID VendorGuid;
if (InfoEntry == NULL) {
@@ -363,12 +364,13 @@ SmmVariableGetStatistics (
return EFI_UNSUPPORTED;
}
- StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + StrSize (VariableInfo->Name);
+ StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY);
if (*InfoSize < StatisticsInfoSize) {
*InfoSize = StatisticsInfoSize;
return EFI_BUFFER_TOO_SMALL;
}
InfoName = (CHAR16 *)(InfoEntry + 1);
+ InfoNameMaxSize = (*InfoSize - sizeof (VARIABLE_INFO_ENTRY));
CopyGuid (&VendorGuid, &InfoEntry->VendorGuid);
@@ -376,8 +378,14 @@ SmmVariableGetStatistics (
//
// Return the first variable info
//
+ NameSize = StrSize (VariableInfo->Name);
+ StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + NameSize;
+ if (*InfoSize < StatisticsInfoSize) {
+ *InfoSize = StatisticsInfoSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));
- CopyMem (InfoName, VariableInfo->Name, StrSize (VariableInfo->Name));
+ CopyMem (InfoName, VariableInfo->Name, NameSize);
*InfoSize = StatisticsInfoSize;
return EFI_SUCCESS;
}
@@ -387,9 +395,9 @@ SmmVariableGetStatistics (
//
while (VariableInfo != NULL) {
if (CompareGuid (&VariableInfo->VendorGuid, &VendorGuid)) {
- NameLength = StrSize (VariableInfo->Name);
- if (NameLength == StrSize (InfoName)) {
- if (CompareMem (VariableInfo->Name, InfoName, NameLength) == 0) {
+ NameSize = StrSize (VariableInfo->Name);
+ if (NameSize <= InfoNameMaxSize) {
+ if (CompareMem (VariableInfo->Name, InfoName, NameSize) == 0) {
//
// Find the match one
//
@@ -409,14 +417,15 @@ SmmVariableGetStatistics (
//
// Output the new variable info
//
- StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + StrSize (VariableInfo->Name);
+ NameSize = StrSize (VariableInfo->Name);
+ StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + NameSize;
if (*InfoSize < StatisticsInfoSize) {
*InfoSize = StatisticsInfoSize;
return EFI_BUFFER_TOO_SMALL;
}
CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));
- CopyMem (InfoName, VariableInfo->Name, StrSize (VariableInfo->Name));
+ CopyMem (InfoName, VariableInfo->Name, NameSize);
*InfoSize = StatisticsInfoSize;
return EFI_SUCCESS;
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] MdeModulePkg VariableSmm: Check InfoSize correctly
2016-12-09 8:47 [PATCH] MdeModulePkg VariableSmm: Check InfoSize correctly Star Zeng
@ 2016-12-11 13:28 ` Yao, Jiewen
0 siblings, 0 replies; 2+ messages in thread
From: Yao, Jiewen @ 2016-12-11 13:28 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, December 9, 2016 4:48 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] MdeModulePkg VariableSmm: Check InfoSize correctly
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=290
>
> Current SmmVariableGetStatistics() in VariableSmm.c is always
> checking input InfoSize against the first variable info,
> it is incorrect.
>
> For instance, there are three variables.
> BootOrder
> Boot0000
> Boot0001
>
> If the input InfoEntry is holding the second variable info (Boot0000)
> and InfoSize is sizeof (VARIABLE_INFO_ENTRY) + StrSize (L"Boot0000"),
> current code will return EFI_BUFFER_TOO_SMALL, but it should return
> the third variable info (Boot0001).
>
> This patch is to refine the code logic.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> .../Universal/Variable/RuntimeDxe/VariableSmm.c | 25
> +++++++++++++++-------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index eafb53322e8c..85158d8b46ae 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -349,9 +349,10 @@ SmmVariableGetStatistics (
> )
> {
> VARIABLE_INFO_ENTRY *VariableInfo;
> - UINTN NameLength;
> + UINTN NameSize;
> UINTN
> StatisticsInfoSize;
> CHAR16 *InfoName;
> + UINTN
> InfoNameMaxSize;
> EFI_GUID VendorGuid;
>
> if (InfoEntry == NULL) {
> @@ -363,12 +364,13 @@ SmmVariableGetStatistics (
> return EFI_UNSUPPORTED;
> }
>
> - StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + StrSize
> (VariableInfo->Name);
> + StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY);
> if (*InfoSize < StatisticsInfoSize) {
> *InfoSize = StatisticsInfoSize;
> return EFI_BUFFER_TOO_SMALL;
> }
> InfoName = (CHAR16 *)(InfoEntry + 1);
> + InfoNameMaxSize = (*InfoSize - sizeof (VARIABLE_INFO_ENTRY));
>
> CopyGuid (&VendorGuid, &InfoEntry->VendorGuid);
>
> @@ -376,8 +378,14 @@ SmmVariableGetStatistics (
> //
> // Return the first variable info
> //
> + NameSize = StrSize (VariableInfo->Name);
> + StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + NameSize;
> + if (*InfoSize < StatisticsInfoSize) {
> + *InfoSize = StatisticsInfoSize;
> + return EFI_BUFFER_TOO_SMALL;
> + }
> CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));
> - CopyMem (InfoName, VariableInfo->Name, StrSize (VariableInfo->Name));
> + CopyMem (InfoName, VariableInfo->Name, NameSize);
> *InfoSize = StatisticsInfoSize;
> return EFI_SUCCESS;
> }
> @@ -387,9 +395,9 @@ SmmVariableGetStatistics (
> //
> while (VariableInfo != NULL) {
> if (CompareGuid (&VariableInfo->VendorGuid, &VendorGuid)) {
> - NameLength = StrSize (VariableInfo->Name);
> - if (NameLength == StrSize (InfoName)) {
> - if (CompareMem (VariableInfo->Name, InfoName, NameLength) == 0)
> {
> + NameSize = StrSize (VariableInfo->Name);
> + if (NameSize <= InfoNameMaxSize) {
> + if (CompareMem (VariableInfo->Name, InfoName, NameSize) == 0) {
> //
> // Find the match one
> //
> @@ -409,14 +417,15 @@ SmmVariableGetStatistics (
> //
> // Output the new variable info
> //
> - StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + StrSize
> (VariableInfo->Name);
> + NameSize = StrSize (VariableInfo->Name);
> + StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + NameSize;
> if (*InfoSize < StatisticsInfoSize) {
> *InfoSize = StatisticsInfoSize;
> return EFI_BUFFER_TOO_SMALL;
> }
>
> CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));
> - CopyMem (InfoName, VariableInfo->Name, StrSize (VariableInfo->Name));
> + CopyMem (InfoName, VariableInfo->Name, NameSize);
> *InfoSize = StatisticsInfoSize;
>
> return EFI_SUCCESS;
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-12-11 13:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-09 8:47 [PATCH] MdeModulePkg VariableSmm: Check InfoSize correctly Star Zeng
2016-12-11 13:28 ` Yao, Jiewen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox