Hi Eric,
I think this patch still needs some work because it isn't enforcing the
min/max of signed integers. In the below example, the following error is
generated:
typedef struct {
UINT8 Nothing[1];
} MY_EFI_VARSTORE_DATA;
numeric varid = MY_EFI_VARSTORE_DATA.Nothing,
prompt = STRING_TOKEN(1),
help = STRING_TOKEN(2),
flags = DISPLAY_INT_DEC,
minimum = 0,
maximum = 200,
step = 1,
endnumeric;
ERROR 12288: Maximum can't be less than Minimum
: invalid parameter
The patch appears to not be bounds checking the values against the maximum
value that can be stored in the type. From limits.h:
#define _I8_MIN (-127i8 - 1) /* minimum signed 8 bit value */
#define _I8_MAX 127i8 /* maximum signed 8 bit value */
numeric varid = MY_EFI_VARSTORE_DATA.Nothing,
prompt = STRING_TOKEN(1),
help = STRING_TOKEN(2),
flags = DISPLAY_INT_DEC,
minimum = -100,
maximum = 200,
step = 1,
endnumeric;
And this is allowed to compile successfully even though 200 is above the
maximum 127 that can be stored in a signed integer.
Best Regards,
Aaron
From: Eric Dong <eric.d...@intel.com>
To: edk2-devel@lists.sourceforge.net, liming....@intel.com,
Date: 04/29/2015 10:57 PM
Subject: [edk2] [Patch 1/2] BaseTools: Enhance the check for
numeric opcode with EFI_IFR_DISPLAY_INT_DEC attribute.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong <eric.d...@intel.com>
---
BaseTools/Source/C/VfrCompile/VfrFormPkg.h | 8 ++
BaseTools/Source/C/VfrCompile/VfrSyntax.g | 170
+++++++++++++++++++++++++----
2 files changed, 156 insertions(+), 22 deletions(-)
diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
index 3d2def8..e64ef1b 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
@@ -308,10 +308,14 @@ public:
}
VOID UpdateHeader (IN EFI_IFR_OP_HEADER *Header) {
mHeader = Header;
}
+
+ UINT8 GetOpCode () {
+ return mHeader->OpCode;
+ }
};
extern UINT8 gScopeCount;
/*
@@ -1356,10 +1360,14 @@ public:
} else {
mNumeric->Flags = LFlags;
}
return VFR_RETURN_SUCCESS;
}
+
+ UINT8 GetNumericFlags () {
+ return mNumeric->Flags;
+ }
};
class CIfrOneOf : public CIfrObj, public CIfrOpHeader, public
CIfrQuestionHeader, public CIfrMinMaxStepData {
private:
EFI_IFR_ONE_OF *mOneOf;
diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 2255d6f..d014aff 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -1394,24 +1394,40 @@ vfrQuestionDataFieldName [EFI_QUESTION_ID &QId,
UINT32 &Mask, CHAR8 *&VarIdStr,
;
vfrConstantValueField[UINT8 Type] > [EFI_IFR_TYPE_VALUE Value] :
<<
EFI_GUID Guid;
+ BOOLEAN Negative = FALSE;
>>
+ {
+ "\-" << Negative = TRUE;
>>
+ }
N1:Number <<
switch ($Type) {
case
EFI_IFR_TYPE_NUM_SIZE_8 :
- $Value.u8 =
_STOU8(N1->getText(), N1->getLine());
+ $Value.u8 =
_STOU8(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u8 =
~$Value.u8 + 1;
+ }
break;
case
EFI_IFR_TYPE_NUM_SIZE_16 :
- $Value.u16 =
_STOU16(N1->getText(), N1->getLine());
+ $Value.u16 =
_STOU16(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u16 =
~$Value.u16 + 1;
+ }
break;
case
EFI_IFR_TYPE_NUM_SIZE_32 :
$Value.u32 =
_STOU32(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u32 =
~$Value.u32 + 1;
+ }
break;
case
EFI_IFR_TYPE_NUM_SIZE_64 :
$Value.u64 =
_STOU64(N1->getText(), N1->getLine());
+ if (Negative) {
+ $Value.u64 =
~$Value.u64 + 1;
+ }
break;
case
EFI_IFR_TYPE_BOOLEAN :
$Value.b =
_STOU8(N1->getText(), N1->getLine());
break;
case
EFI_IFR_TYPE_STRING :
@@ -1531,20 +1547,53 @@ vfrStatementDefault :
EFI_DEFAULT_ID DefaultId =
EFI_HII_DEFAULT_CLASS_STANDARD;
CHAR8 *VarStoreName = NULL;
EFI_VFR_VARSTORE_TYPE VarStoreType = EFI_VFR_VARSTORE_INVALID;
UINT32 Size = 0;
EFI_GUID *VarGuid = NULL;
+ CIfrNumeric *NumericQst = NULL;
>>
D:Default
(
(
"=" vfrConstantValueField[_GET_CURRQEST_DATATYPE()] > [Val] ","
<<
if
(gCurrentMinMaxData != NULL && gCurrentMinMaxData->IsNumericOpcode()) {
//check default
value is valid for Numeric Opcode
- if (Val.u64 <
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE()) || Val.u64 >
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE())) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ NumericQst =
(CIfrNumeric *) gCurrentQuestion;
+ if
((NumericQst->GetNumericFlags() & EFI_IFR_DISPLAY) == 0) {
+ switch
(_GET_CURRQEST_DATATYPE()) {
+ case
EFI_IFR_TYPE_NUM_SIZE_8:
+ if (((INT8)
Val.u8 < (INT8) gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE()))
||
+ ((INT8)
Val.u8 > (INT8) gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE())))
{
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_16:
+ if
(((INT16) Val.u16 < (INT16)
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE())) ||
+ ((INT16) Val.u16 > (INT16)
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE()))) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_32:
+ if
(((INT32) Val.u32 < (INT32)
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE())) ||
+ ((INT32) Val.u32 > (INT32)
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE()))) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_64:
+ if
(((INT64) Val.u64 < (INT64)
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE())) ||
+ ((INT64) Val.u64 > (INT64)
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE()))) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
+ break;
+ default:
+ break;
+ }
+ } else {
+ if (Val.u64 <
gCurrentMinMaxData->GetMinData(_GET_CURRQEST_DATATYPE()) || Val.u64 >
gCurrentMinMaxData->GetMaxData(_GET_CURRQEST_DATATYPE())) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, D->getLine(), "Numeric default value must
be between MinValue and MaxValue.");
+ }
}
}
if
(_GET_CURRQEST_DATATYPE() == EFI_IFR_TYPE_OTHER) {
_PCATCH
(VFR_RETURN_FATAL_ERROR, D->getLine(), "Default data type error.");
Size = sizeof
(EFI_IFR_TYPE_VALUE);
@@ -2218,45 +2267,122 @@ vfrSetMinMaxStep[CIfrMinMaxStepData & MMSDObj] :
<<
UINT64 MaxU8 = 0, MinU8 = 0, StepU8 = 0;
UINT32 MaxU4 = 0, MinU4 = 0, StepU4 = 0;
UINT16 MaxU2 = 0, MinU2 = 0, StepU2 = 0;
UINT8 MaxU1 = 0, MinU1 = 0, StepU1 = 0;
+ BOOLEAN IntDecStyle = FALSE;
+ CIfrNumeric *NObj = (CIfrNumeric *) (&MMSDObj);
+ if ((NObj->GetOpCode() == EFI_IFR_NUMERIC_OP) &&
((NObj->GetNumericFlags() & EFI_IFR_DISPLAY) == 0)) {
+ IntDecStyle = TRUE;
+ }
+ BOOLEAN MinNegative = FALSE;
+ BOOLEAN MaxNegative = FALSE;
>>
- Minimum "=" I:Number ","
- <<
+ Minimum "="
+ {
+ "\-" << MinNegative =
TRUE; >>
+ }
+ I:Number "," <<
+ if
(!IntDecStyle && MinNegative) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, I->getLine(), "\"-\" can't be used when not
in int decimal type. ");
+ }
switch
(_GET_CURRQEST_DATATYPE()) {
- case
EFI_IFR_TYPE_NUM_SIZE_64 : MinU8 = _STOU64(I->getText(), I->getLine());
break;
- case
EFI_IFR_TYPE_NUM_SIZE_32 : MinU4 = _STOU32(I->getText(), I->getLine());
break;
- case
EFI_IFR_TYPE_NUM_SIZE_16 : MinU2 = _STOU16(I->getText(), I->getLine());
break;
- case
EFI_IFR_TYPE_NUM_SIZE_8 : MinU1 = _STOU8(I->getText(), I->getLine());
break;
+ case
EFI_IFR_TYPE_NUM_SIZE_64 :
+ MinU8 =
_STOU64(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU8 =
~MinU8 + 1;
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_32 :
+ MinU4 =
_STOU32(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU4 =
~MinU4 + 1;
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_16 :
+ MinU2 =
_STOU16(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU2 =
~MinU2 + 1;
+ }
+ break;
+ case
EFI_IFR_TYPE_NUM_SIZE_8 :
+ MinU1 =
_STOU8(I->getText(), I->getLine());
+ if
(MinNegative) {
+ MinU1 =
~MinU1 + 1;
+ }
+ break;
}
>>
- Maximum "=" A:Number ","
- <<
+ Maximum "="
+ {
+ "\-" << MaxNegative =
TRUE; >>
+ }
+ A:Number "," <<
+ if
(!IntDecStyle && MaxNegative) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "\"-\" can't be used when not
in int decimal type. ");
+ }
+
switch
(_GET_CURRQEST_DATATYPE()) {
case
EFI_IFR_TYPE_NUM_SIZE_64 :
MaxU8 =
_STOU64(A->getText(), A->getLine());
- if (MaxU8 <
MinU8) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ if
(MaxNegative) {
+ MaxU8 =
~MaxU8 + 1;
+ }
+ if
(IntDecStyle) {
+ if ((INT64)
MaxU8 < (INT64) MinU8) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU8 <
MinU8) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
case
EFI_IFR_TYPE_NUM_SIZE_32 :
- MaxU4 =
_STOU32(A->getText(), A->getLine());
- if (MaxU4 <
MinU4) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ MaxU4 =
_STOU32(A->getText(), A->getLine());
+ if
(MaxNegative) {
+ MaxU4 =
~MaxU4 + 1;
+ }
+ if
(IntDecStyle) {
+ if ((INT32)
MaxU4 < (INT32) MinU4) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU4 <
MinU4) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
case
EFI_IFR_TYPE_NUM_SIZE_16 :
MaxU2 =
_STOU16(A->getText(), A->getLine());
- if (MaxU2 <
MinU2) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ if
(MaxNegative) {
+ MaxU2 =
~MaxU2 + 1;
+ }
+
+ if
(IntDecStyle) {
+ if ((INT16)
MaxU2 < (INT16) MinU2) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU2 <
MinU2) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
case
EFI_IFR_TYPE_NUM_SIZE_8 :
- MaxU1 =
_STOU8(A->getText(), A->getLine());
- if (MaxU1 <
MinU1) {
- _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ MaxU1 =
_STOU8(A->getText(), A->getLine());
+ if
(MaxNegative) {
+ MaxU1 =
~MaxU1 + 1;
+ }
+ if
(IntDecStyle) {
+ if ((INT8)
MaxU1 < (INT8) MinU1) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
+ } else {
+ if (MaxU1 <
MinU1) {
+ _PCATCH
(VFR_RETURN_INVALID_PARAMETER, A->getLine(), "Maximum can't be less than
Minimum");
+ }
}
break;
}
>>
{
--
1.9.5.msysgit.1
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
