[U-Boot] [PATCH v3 0/2] efi_loader: allow writing to FAT
Running UEFI Self-Certifcation Test (SCT) will require to support writing to a FAT file system.
Writing to the FAT root directory from EFI fails because the leading slash of the file path is interpreted as part of the filename.
With the 1st patch we can write to the root directory.
A unit test for writing a new file to the FAT root directory is supplied.
We still cannot write to subdirectories.
v3: Do not throw an error in the unit test if CONFIG_FAT_WRITE is not not set. Drop accepted patch implying CONFIG_FAT_WRITE from patch set.
Heinrich Schuchardt (2): fs: fat: cannot write to subdirectories efi_selftest: test writing to file
fs/fat/fat_write.c | 16 ++++- lib/efi_selftest/efi_selftest_block_device.c | 70 ++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-)
fs_fat_write() is not able to write to subdirectories.
Currently if a filepath with a leading slash is passed, the slash is treated as part of the filename to be created in the root directory.
Strip leading (back-)slashes.
Check that the remaining filename does not contain any illegal characters (<>:"/|?*). This way we will throw an error when trying to write to a subdirectory.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- fs/fat/fat_write.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 5ca8fcda73..c5aee519b7 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -908,9 +908,11 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, volume_info volinfo; fsdata datablock; fsdata *mydata = &datablock; - int cursect; + int cursect, i; int ret = -1, name_len; char l_filename[VFAT_MAXLEN_BYTES]; + char bad[2] = " "; + const char illegal[] = "<>:"/\|?*";
*actwrite = size; dir_curclust = 0; @@ -970,6 +972,18 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, } dentptr = (dir_entry *) do_fat_read_at_block;
+ /* Strip leading (back-)slashes */ + while ISDIRDELIM(*filename) + ++filename; + /* Check that the filename is valid */ + for (i = 0; i < strlen(illegal); ++i) { + *bad = illegal[i]; + if (strstr(filename, bad)) { + printf("FAT: illegal filename (%s)\n", filename); + return -1; + } + } + name_len = strlen(filename); if (name_len >= VFAT_MAXLEN_BYTES) name_len = VFAT_MAXLEN_BYTES - 1;
Hi Heinrich,
On 1 July 2018 at 17:41, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
fs_fat_write() is not able to write to subdirectories.
Currently if a filepath with a leading slash is passed, the slash is treated as part of the filename to be created in the root directory.
Strip leading (back-)slashes.
Check that the remaining filename does not contain any illegal characters (<>:"/|?*). This way we will throw an error when trying to write to a subdirectory.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
fs/fat/fat_write.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
It would be great if we have filesystem tests. We do have the fs-test.sh script but it is not using pytest, so best not to build on it.
Regards, Simon
On 07/09/2018 04:35 AM, Simon Glass wrote:
Hi Heinrich,
On 1 July 2018 at 17:41, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
fs_fat_write() is not able to write to subdirectories.
Currently if a filepath with a leading slash is passed, the slash is treated as part of the filename to be created in the root directory.
Strip leading (back-)slashes.
Check that the remaining filename does not contain any illegal characters (<>:"/|?*). This way we will throw an error when trying to write to a subdirectory.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
fs/fat/fat_write.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
It would be great if we have filesystem tests. We do have the fs-test.sh script but it is not using pytest, so best not to build on it.
There is a write test for FAT in lib/efi_selftest/efi_selftest_block_device.c. See patch "efi_selftest: test writing to file".
I agree having tests independent of the EFI subsystem would be a good idea.
Best regards
Heinrich
Provide a unit test for writing to a FAT file system. Add some additional comments in block device unit test.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_selftest/efi_selftest_block_device.c | 70 ++++++++++++++++++++ 1 file changed, 70 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c index 4af8bd8cb2..b82e405030 100644 --- a/lib/efi_selftest/efi_selftest_block_device.c +++ b/lib/efi_selftest/efi_selftest_block_device.c @@ -309,11 +309,14 @@ static int execute(void) efi_uintn_t buf_size; char buf[16] __aligned(ARCH_DMA_MINALIGN);
+ /* Connect controller to virtual disk */ ret = boottime->connect_controller(disk_handle, NULL, NULL, 1); if (ret != EFI_SUCCESS) { efi_st_error("Failed to connect controller\n"); return EFI_ST_FAILURE; } + + /* Get the handle for the partition */ ret = boottime->locate_handle_buffer( BY_PROTOCOL, &guid_device_path, NULL, &no_handles, &handles); @@ -347,6 +350,8 @@ static int execute(void) efi_st_error("Partition handle not found\n"); return EFI_ST_FAILURE; } + + /* Open the simple file system protocol */ ret = boottime->open_protocol(handle_partition, &guid_simple_file_system_protocol, (void **)&file_system, NULL, NULL, @@ -355,6 +360,8 @@ static int execute(void) efi_st_error("Failed to open simple file system protocol\n"); return EFI_ST_FAILURE; } + + /* Open volume */ ret = file_system->open_volume(file_system, &root); if (ret != EFI_SUCCESS) { efi_st_error("Failed to open volume\n"); @@ -377,6 +384,8 @@ static int execute(void) "Wrong volume label '%ps', expected 'U-BOOT TEST'\n", system_info.info.volume_label); } + + /* Read file */ ret = root->open(root, &file, (s16 *)L"hello.txt", EFI_FILE_MODE_READ, 0); if (ret != EFI_SUCCESS) { @@ -389,6 +398,11 @@ static int execute(void) efi_st_error("Failed to read file\n"); return EFI_ST_FAILURE; } + if (buf_size != 13) { + efi_st_error("Wrong number of bytes read: %u\n", + (unsigned int)buf_size); + return EFI_ST_FAILURE; + } if (efi_st_memcmp(buf, "Hello world!", 12)) { efi_st_error("Unexpected file content\n"); return EFI_ST_FAILURE; @@ -398,6 +412,62 @@ static int execute(void) efi_st_error("Failed to close file\n"); return EFI_ST_FAILURE; } + +#ifdef CONFIG_FAT_WRITE + /* Write file */ + ret = root->open(root, &file, (s16 *)L"u-boot.txt", + EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE, 0); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to open file\n"); + return EFI_ST_FAILURE; + } + buf_size = 7; + boottime->set_mem(buf, sizeof(buf), 0); + boottime->copy_mem(buf, "U-Boot", buf_size); + ret = file->write(file, &buf_size, buf); + if (ret != EFI_SUCCESS || buf_size != 7) { + efi_st_error("Failed to write file\n"); + return EFI_ST_FAILURE; + } + ret = file->close(file); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to close file\n"); + return EFI_ST_FAILURE; + } + + /* Verify file */ + boottime->set_mem(buf, sizeof(buf), 0); + ret = root->open(root, &file, (s16 *)L"u-boot.txt", EFI_FILE_MODE_READ, + 0); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to open file\n"); + return EFI_ST_FAILURE; + } + buf_size = sizeof(buf) - 1; + ret = file->read(file, &buf_size, buf); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to read file\n"); + return EFI_ST_FAILURE; + } + if (buf_size != 7) { + efi_st_error("Wrong number of bytes read: %u\n", + (unsigned int)buf_size); + return EFI_ST_FAILURE; + } + if (efi_st_memcmp(buf, "U-Boot", 7)) { + efi_st_error("Unexpected file content %s\n", buf); + return EFI_ST_FAILURE; + } + ret = file->close(file); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to close file\n"); + return EFI_ST_FAILURE; + } +#else + efi_st_todo("CONFIG_FAT_WRITE is not set\n"); +#endif /* CONFIG_FAT_WRITE */ + + /* Close volume */ ret = root->close(root); if (ret != EFI_SUCCESS) { efi_st_error("Failed to close volume\n");
participants (2)
-
Heinrich Schuchardt -
Simon Glass