[U-Boot] [RFC 1/1] efi_loader: check parameters of efi_file_open()
Check the parameters of efi_file_open().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- Hello Takahiro,
this patch is necessary to become more UEFI compliant. But it interferes with your FAT patch series.
You might integrate the changes into the next version of you patch series. Otherwise I keep the patch until your series is merged.
Best regards
Heinrich --- lib/efi_loader/efi_file.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 07696a8f56..8f04943670 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -202,15 +202,37 @@ static efi_status_t EFIAPI efi_file_open(struct efi_file_handle *file, s16 *file_name, u64 open_mode, u64 attributes) { struct file_handle *fh = to_fh(file); + efi_status_t ret;
EFI_ENTRY("%p, %p, "%ls", %llx, %llu", file, new_handle, file_name, open_mode, attributes);
- *new_handle = file_open(fh->fs, fh, file_name, open_mode); - if (!*new_handle) - return EFI_EXIT(EFI_NOT_FOUND); + /* Check parameters */ + if (!file || !file || !file_name) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + if (open_mode != EFI_FILE_MODE_READ && + open_mode != (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE) && + open_mode != (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | + EFI_FILE_MODE_CREATE)) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + if ((!(open_mode & EFI_FILE_MODE_CREATE) && attributes) || + (attributes & (EFI_FILE_READ_ONLY | ~EFI_FILE_VALID_ATTR))) { + ret = EFI_INVALID_PARAMETER; + goto out; + }
- return EFI_EXIT(EFI_SUCCESS); + /* Open file */ + *new_handle = file_open(fh->fs, fh, file_name, open_mode); + if (*new_handle) + ret = EFI_SUCCESS; + else + ret = EFI_NOT_FOUND; +out: + return EFI_EXIT(ret); }
static efi_status_t file_close(struct file_handle *fh)
On Sun, Jul 22, 2018 at 01:34:13PM +0200, Heinrich Schuchardt wrote:
Check the parameters of efi_file_open().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Hello Takahiro,
this patch is necessary to become more UEFI compliant. But it interferes with your FAT patch series.
You might integrate the changes into the next version of you patch series. Otherwise I keep the patch until your series is merged.
I still believe, as I mentioned elsewhere, that such kind of patches be compiled into a separate (follow-up) patch series.
Speaking of your specific patch, it looks good as a whole, please let me advise you that a section number in UEFI specification or UEFI SCT test case number be included in your comment wherever possible which would make it easier for not only me but also others to assure your changes.
Best regards
Heinrich
lib/efi_loader/efi_file.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 07696a8f56..8f04943670 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -202,15 +202,37 @@ static efi_status_t EFIAPI efi_file_open(struct efi_file_handle *file, s16 *file_name, u64 open_mode, u64 attributes) { struct file_handle *fh = to_fh(file);
efi_status_t ret;
EFI_ENTRY("%p, %p, "%ls", %llx, %llu", file, new_handle, file_name, open_mode, attributes);
- *new_handle = file_open(fh->fs, fh, file_name, open_mode);
- if (!*new_handle)
return EFI_EXIT(EFI_NOT_FOUND);
- /* Check parameters */
- if (!file || !file || !file_name) {
nit: duplicated !file
ret = EFI_INVALID_PARAMETER;
Strangely, EFI_INVALID_PARAMETER is not listed in "Status Codes Returned" at section 13.5 File Protocol, p.524.
goto out;- }
- if (open_mode != EFI_FILE_MODE_READ &&
open_mode != (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE) &&open_mode != (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |EFI_FILE_MODE_CREATE)) {
(open_mode | CREATE) && !(open_mode | WRITE) would be simpler.
ret = EFI_INVALID_PARAMETER;goto out;- }
- if ((!(open_mode & EFI_FILE_MODE_CREATE) && attributes) ||
OK,
(attributes & (EFI_FILE_READ_ONLY | ~EFI_FILE_VALID_ATTR))) {
Why is EFI_FILE_READ_ONLY not allowed?
ret = EFI_INVALID_PARAMETER;goto out;- }
- return EFI_EXIT(EFI_SUCCESS);
- /* Open file */
- *new_handle = file_open(fh->fs, fh, file_name, open_mode);
- if (*new_handle)
ret = EFI_SUCCESS;- else
ret = EFI_NOT_FOUND;
Should we always return NOT_FOUND here? (No choices, probably)
-Takahiro AKASHI
+out:
- return EFI_EXIT(ret);
}
static efi_status_t file_close(struct file_handle *fh)
2.18.0
participants (2)
-
AKASHI, Takahiro -
Heinrich Schuchardt