| Message ID | 1466153906-23285-1-git-send-email-ivan.hu@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On 17/06/16 11:58, Ivan Hu wrote: > Some firmwares (mostly on AMI Bios), the implementation limitation was seen > that the VariableNameSize of the GetNextVariableName should lager or equal to 2. > They believe that it should at least have a Null-terminated(2 bytes) for the > name buffer. > > So test getnextvariablename with VariableNameSize 0 and 1 will get the > EFI_INVALID_PARAMETER, not expected EFI_BUFFER_TOO_SMALL. > > UEFI spec doesn't describe it clearly, VariableName is the Null-treminated > string, so the EFI_INVALID_PARAMETER seems also a resonable return when > VariableNameSize smaller than 2. > Before the spec has a clearly description, for the fwts getnextvariablename > buffer too small test(getnextvariable_test4), > 1. add the VariableNameSize 2 for the test, if VariableNameSize 2 without > returning EFI_BUFFER_TOO_SMALL, then test fail. > 2. if VariableNameSize is 0 or 1, allow the extra return EFI_INVALID_PARAMETER. > > Signed-off-by: Ivan Hu <ivan.hu@canonical.com> > --- > src/uefi/uefirtvariable/uefirtvariable.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c > index 0514805..aca0202 100644 > --- a/src/uefi/uefirtvariable/uefirtvariable.c > +++ b/src/uefi/uefirtvariable/uefirtvariable.c > @@ -812,8 +812,7 @@ static int getnextvariable_test4(fwts_framework *fw) > goto err; > } > > - /* No first result can be 0 or 1 byte in size. */ > - for (i = 0; i < 2; i++) { > + for (i = 0; i < 3; i++) { > variablenamesize = i; > getnextvariablename.VariableNameSize = &variablenamesize; > > @@ -831,6 +830,8 @@ static int getnextvariable_test4(fwts_framework *fw) > * EFI_NOT_FOUND at this point. > */ > if (ioret != -1 || status != EFI_BUFFER_TOO_SMALL) { > + if (i != 2 && status == EFI_INVALID_PARAMETER) > + continue; > fwts_failed(fw, LOG_LEVEL_HIGH, > "UEFIRuntimeGetNextVariableName", > "Expected EFI_BUFFER_TOO_SMALL with small " > Given that the spec is not clear, this seems like the best approach. Acked-by: Colin Ian King <colin.king@canonical.com>
On 2016-06-17 05:03 PM, Colin Ian King wrote: > On 17/06/16 11:58, Ivan Hu wrote: >> Some firmwares (mostly on AMI Bios), the implementation limitation was seen >> that the VariableNameSize of the GetNextVariableName should lager or equal to 2. >> They believe that it should at least have a Null-terminated(2 bytes) for the >> name buffer. >> >> So test getnextvariablename with VariableNameSize 0 and 1 will get the >> EFI_INVALID_PARAMETER, not expected EFI_BUFFER_TOO_SMALL. >> >> UEFI spec doesn't describe it clearly, VariableName is the Null-treminated >> string, so the EFI_INVALID_PARAMETER seems also a resonable return when >> VariableNameSize smaller than 2. >> Before the spec has a clearly description, for the fwts getnextvariablename >> buffer too small test(getnextvariable_test4), >> 1. add the VariableNameSize 2 for the test, if VariableNameSize 2 without >> returning EFI_BUFFER_TOO_SMALL, then test fail. >> 2. if VariableNameSize is 0 or 1, allow the extra return EFI_INVALID_PARAMETER. >> >> Signed-off-by: Ivan Hu <ivan.hu@canonical.com> >> --- >> src/uefi/uefirtvariable/uefirtvariable.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c >> index 0514805..aca0202 100644 >> --- a/src/uefi/uefirtvariable/uefirtvariable.c >> +++ b/src/uefi/uefirtvariable/uefirtvariable.c >> @@ -812,8 +812,7 @@ static int getnextvariable_test4(fwts_framework *fw) >> goto err; >> } >> >> - /* No first result can be 0 or 1 byte in size. */ >> - for (i = 0; i < 2; i++) { >> + for (i = 0; i < 3; i++) { >> variablenamesize = i; >> getnextvariablename.VariableNameSize = &variablenamesize; >> >> @@ -831,6 +830,8 @@ static int getnextvariable_test4(fwts_framework *fw) >> * EFI_NOT_FOUND at this point. >> */ >> if (ioret != -1 || status != EFI_BUFFER_TOO_SMALL) { >> + if (i != 2 && status == EFI_INVALID_PARAMETER) >> + continue; >> fwts_failed(fw, LOG_LEVEL_HIGH, >> "UEFIRuntimeGetNextVariableName", >> "Expected EFI_BUFFER_TOO_SMALL with small " >> > Given that the spec is not clear, this seems like the best approach. > > Acked-by: Colin Ian King <colin.king@canonical.com> > Acked-by: Alex Hung <alex.hung@canonical.com>
diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c index 0514805..aca0202 100644 --- a/src/uefi/uefirtvariable/uefirtvariable.c +++ b/src/uefi/uefirtvariable/uefirtvariable.c @@ -812,8 +812,7 @@ static int getnextvariable_test4(fwts_framework *fw) goto err; } - /* No first result can be 0 or 1 byte in size. */ - for (i = 0; i < 2; i++) { + for (i = 0; i < 3; i++) { variablenamesize = i; getnextvariablename.VariableNameSize = &variablenamesize; @@ -831,6 +830,8 @@ static int getnextvariable_test4(fwts_framework *fw) * EFI_NOT_FOUND at this point. */ if (ioret != -1 || status != EFI_BUFFER_TOO_SMALL) { + if (i != 2 && status == EFI_INVALID_PARAMETER) + continue; fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextVariableName", "Expected EFI_BUFFER_TOO_SMALL with small "
Some firmwares (mostly on AMI Bios), the implementation limitation was seen that the VariableNameSize of the GetNextVariableName should lager or equal to 2. They believe that it should at least have a Null-terminated(2 bytes) for the name buffer. So test getnextvariablename with VariableNameSize 0 and 1 will get the EFI_INVALID_PARAMETER, not expected EFI_BUFFER_TOO_SMALL. UEFI spec doesn't describe it clearly, VariableName is the Null-treminated string, so the EFI_INVALID_PARAMETER seems also a resonable return when VariableNameSize smaller than 2. Before the spec has a clearly description, for the fwts getnextvariablename buffer too small test(getnextvariable_test4), 1. add the VariableNameSize 2 for the test, if VariableNameSize 2 without returning EFI_BUFFER_TOO_SMALL, then test fail. 2. if VariableNameSize is 0 or 1, allow the extra return EFI_INVALID_PARAMETER. Signed-off-by: Ivan Hu <ivan.hu@canonical.com> --- src/uefi/uefirtvariable/uefirtvariable.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)