[U-Boot] [RFC 0/3] 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.
I am still unhappy that we cannot write to subdirectories. I therefore mark the series as RFC.
Heinrich Schuchardt (3): fs: fat: cannot write to subdirectories efi_selftest: imply FAT, FAT_WRITE efi_selftest: test writing to file
fs/fat/fat_write.c | 18 ++++++- lib/efi_selftest/Kconfig | 2 + lib/efi_selftest/efi_selftest_block_device.c | 56 ++++++++++++++++++++ 3 files changed, 75 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 | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 5ca8fcda73c..f32745febb4 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,20 @@ 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 (*filename == '/') + ++filename; + while (*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;
On Sat, 26 May 2018 10:34:45 +0200 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 | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 5ca8fcda73c..f32745febb4 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,20 @@ 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 (*filename == '/')
++filename;- while (*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;
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
efi_selftest_block_device accesses a FAT file system.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_selftest/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/efi_selftest/Kconfig b/lib/efi_selftest/Kconfig index 3b5f3a1230b..59f9f36801c 100644 --- a/lib/efi_selftest/Kconfig +++ b/lib/efi_selftest/Kconfig @@ -1,6 +1,8 @@ config CMD_BOOTEFI_SELFTEST bool "Allow booting an EFI efi_selftest" depends on CMD_BOOTEFI + imply FAT + imply FAT_WRITE help This adds an EFI test application to U-Boot that can be executed with the 'bootefi selftest' command. It provides extended tests of
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_selftest/efi_selftest_block_device.c | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c index 4af8bd8cb28..9209ce678ee 100644 --- a/lib/efi_selftest/efi_selftest_block_device.c +++ b/lib/efi_selftest/efi_selftest_block_device.c @@ -377,6 +377,7 @@ 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 +390,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; @@ -399,6 +405,56 @@ static int execute(void) return EFI_ST_FAILURE; } ret = root->close(root); + /* 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; + } + ret = root->close(root); + /* 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; + } + ret = root->close(root); if (ret != EFI_SUCCESS) { efi_st_error("Failed to close volume\n"); return EFI_ST_FAILURE;
Heinrich,
On Sat, May 26, 2018 at 10:34:47AM +0200, Heinrich Schuchardt wrote:
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_selftest/efi_selftest_block_device.c | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c index 4af8bd8cb28..9209ce678ee 100644 --- a/lib/efi_selftest/efi_selftest_block_device.c +++ b/lib/efi_selftest/efi_selftest_block_device.c @@ -377,6 +377,7 @@ 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 +390,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;
@@ -399,6 +405,56 @@ static int execute(void) return EFI_ST_FAILURE; } ret = root->close(root);
We should not close 'root' here as
- /* Write file */
- ret = root->open(root, &file, (s16 *)L"u-boot.txt",
EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE, 0);
we will use it here.
- 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;- }
- ret = root->close(root);
ditto
Thanks, -Takahiro AKASHI
- /* 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;- }
- ret = root->close(root); if (ret != EFI_SUCCESS) { efi_st_error("Failed to close volume\n"); return EFI_ST_FAILURE;
-- 2.17.0
On 06/01/2018 09:13 AM, AKASHI, Takahiro wrote:
Heinrich,
On Sat, May 26, 2018 at 10:34:47AM +0200, Heinrich Schuchardt wrote:
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_selftest/efi_selftest_block_device.c | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c index 4af8bd8cb28..9209ce678ee 100644 --- a/lib/efi_selftest/efi_selftest_block_device.c +++ b/lib/efi_selftest/efi_selftest_block_device.c @@ -377,6 +377,7 @@ 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 +390,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;
@@ -399,6 +405,56 @@ static int execute(void) return EFI_ST_FAILURE; } ret = root->close(root);
Your observation is correct. This is why I already sent a revised version of the patch series. I unfortunately missed to put you on copy.
You can find the current patch here:
[PATCH v2 3/3] efi_selftest: test writing to file https://patchwork.ozlabs.org/patch/921166/ https://lists.denx.de/pipermail/u-boot/2018-May/329730.html
Best regards
Heinrich
We should not close 'root' here as
- /* Write file */
- ret = root->open(root, &file, (s16 *)L"u-boot.txt",
EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE, 0);we will use it here.
- 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;- }
- ret = root->close(root);
ditto
Thanks, -Takahiro AKASHI
- /* 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;- }
- ret = root->close(root); if (ret != EFI_SUCCESS) { efi_st_error("Failed to close volume\n"); return EFI_ST_FAILURE;
-- 2.17.0
participants (3)
-
AKASHI, Takahiro -
Heinrich Schuchardt -
Lukasz Majewski