[U-Boot] [PATCH v3 0/2] efi_loader: allow writing to FAT

[U-Boot] [PATCH v3 0/2] efi_loader: allow writing to FAT

[Heinrich Schuchardt]

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(-)

-- 
2.18.0

[U-Boot] [PATCH v3 1/2] fs: fat: cannot write to subdirectories

[Heinrich Schuchardt]

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;
-- 
2.18.0

[U-Boot] [PATCH v3 2/2] efi_selftest: test writing to file

[Heinrich Schuchardt]

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");
-- 
2.18.0

[U-Boot] [PATCH v3 1/2] fs: fat: cannot write to subdirectories

[Simon Glass]

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

[U-Boot] [PATCH v3 1/2] fs: fat: cannot write to subdirectories

[Heinrich Schuchardt]

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