Merge patch series "fs: ext4: implement opendir, readdir, closedir"

Heinrich Schuchardt <heinrich.schuchardt@canonical.com> says:

With this series opendir, readdir, closedir are implemented for ext4.
These functions are needed for the UEFI sub-system to interact with
the ext4 file system.

To reduce code growth the functions are reused to implement the ls
command for ext4.

A memory leak in ext4fs_exists is resolved.

ext4fs_iterate_dir is simplified by removing a redundant pointer copy.

Link: https://lore.kernel.org/r/20241026064048.370062-1-heinrich.schuchardt@canonical.com

fs/ext4/ext4_common.c

@@ -2046,24 +2046,23 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
 	unsigned int fpos = 0;
 	int status;
 	loff_t actread;
-	struct ext2fs_node *diro = (struct ext2fs_node *) dir;
 
 #ifdef DEBUG
 	if (name != NULL)
 		printf("Iterate dir %s\n", name);
 #endif /* of DEBUG */
-	if (!diro->inode_read) {
+	if (!dir->inode_read) {
-		status = ext4fs_read_inode(diro->data, diro->ino, &diro->inode);
+		status = ext4fs_read_inode(dir->data, dir->ino, &dir->inode);
 		if (status == 0)
 			return 0;
 	}
 	/* Search the file.  */
-	while (fpos < le32_to_cpu(diro->inode.size)) {
+	while (fpos < le32_to_cpu(dir->inode.size)) {
 		struct ext2_dirent dirent;
 
-		status = ext4fs_read_file(diro, fpos,
+		status = ext4fs_read_file(dir, fpos,
-					   sizeof(struct ext2_dirent),
+					  sizeof(struct ext2_dirent),
-					   (char *)&dirent, &actread);
+					  (char *)&dirent, &actread);
 		if (status < 0)
 			return 0;
 
@@ -2077,7 +2076,7 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
 			struct ext2fs_node *fdiro;
 			int type = FILETYPE_UNKNOWN;
 
-			status = ext4fs_read_file(diro,
+			status = ext4fs_read_file(dir,
 						  fpos +
 						  sizeof(struct ext2_dirent),
 						  dirent.namelen, filename,
@@ -2089,7 +2088,7 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
 			if (!fdiro)
 				return 0;
 
-			fdiro->data = diro->data;
+			fdiro->data = dir->data;
 			fdiro->ino = le32_to_cpu(dirent.inode);
 
 			filename[dirent.namelen] = '\0';
@@ -2104,7 +2103,7 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
 				else if (dirent.filetype == FILETYPE_REG)
 					type = FILETYPE_REG;
 			} else {
-				status = ext4fs_read_inode(diro->data,
+				status = ext4fs_read_inode(dir->data,
 							   le32_to_cpu
 							   (dirent.inode),
 							   &fdiro->inode);
@@ -2138,35 +2137,6 @@ int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
 					*fnode = fdiro;
 					return 1;
 				}
-			} else {
-				if (fdiro->inode_read == 0) {
-					status = ext4fs_read_inode(diro->data,
-								 le32_to_cpu(
-								 dirent.inode),
-								 &fdiro->inode);
-					if (status == 0) {
-						free(fdiro);
-						return 0;
-					}
-					fdiro->inode_read = 1;
-				}
-				switch (type) {
-				case FILETYPE_DIRECTORY:
-					printf("<DIR> ");
-					break;
-				case FILETYPE_SYMLINK:
-					printf("<SYM> ");
-					break;
-				case FILETYPE_REG:
-					printf("      ");
-					break;
-				default:
-					printf("< ? > ");
-					break;
-				}
-				printf("%10u %s\n",
-				       le32_to_cpu(fdiro->inode.size),
-					filename);
 			}
 			free(fdiro);
 		}

fs/ext4/ext4fs.c

@@ -21,17 +21,36 @@
  */
 
 #include <blk.h>
+#include <div64.h>
+#include <errno.h>
 #include <ext_common.h>
 #include <ext4fs.h>
-#include "ext4_common.h"
-#include <div64.h>
 #include <malloc.h>
 #include <part.h>
 #include <u-boot/uuid.h>
+#include "ext4_common.h"
 
 int ext4fs_symlinknest;
 struct ext_filesystem ext_fs;
 
+/**
+ * struct ext4_dir_stream - ext4 directory stream
+ *
+ * @parent: partition data used by fs layer.
+ * This field must be at the beginning of the structure.
+ * All other fields are private to the ext4 driver.
+ * @root:	root directory node
+ * @dir:	directory node
+ * @dirent:	directory stream entry
+ * @fpos:	file position in directory
+ */
+struct ext4_dir_stream {
+	struct fs_dir_stream parent;
+	char *dirname;
+	struct fs_dirent dirent;
+	unsigned int fpos;
+};
+
 struct ext_filesystem *get_fs(void)
 {
 	return &ext_fs;
@@ -182,39 +201,159 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos,
 	return 0;
 }
 
-int ext4fs_ls(const char *dirname)
+int ext4fs_opendir(const char *dirname, struct fs_dir_stream **dirsp)
 {
-	struct ext2fs_node *dirnode = NULL;
+	struct ext4_dir_stream *dirs;
-	int status;
+	struct ext2fs_node *dir = NULL;
+	int ret;
 
-	if (dirname == NULL)
+	*dirsp = NULL;
-		return 0;
 
-	status = ext4fs_find_file(dirname, &ext4fs_root->diropen, &dirnode,
+	dirs = calloc(1, sizeof(struct ext4_dir_stream));
-				  FILETYPE_DIRECTORY);
+	if (!dirs)
-	if (status != 1) {
+		return -ENOMEM;
-		printf("** Can not find directory. **\n");
+	dirs->dirname = strdup(dirname);
-		if (dirnode)
+	if (!dirs) {
-			ext4fs_free_node(dirnode, &ext4fs_root->diropen);
+		free(dirs);
-		return 1;
+		return -ENOMEM;
 	}
 
-	ext4fs_iterate_dir(dirnode, NULL, NULL, NULL);
+	ret = ext4fs_find_file(dirname, &ext4fs_root->diropen, &dir,
-	ext4fs_free_node(dirnode, &ext4fs_root->diropen);
+			       FILETYPE_DIRECTORY);
+	if (ret == 1) {
+		ret = 0;
+		*dirsp = (struct fs_dir_stream *)dirs;
+	} else {
+		ret = -ENOENT;
+	}
 
-	return 0;
+	if (dir)
+		ext4fs_free_node(dir, &ext4fs_root->diropen);
+
+	return ret;
+}
+
+int ext4fs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp)
+{
+	struct ext4_dir_stream *dirs = (struct ext4_dir_stream *)fs_dirs;
+	struct fs_dirent *dent = &dirs->dirent;
+	struct ext2fs_node *dir = NULL;
+	int ret;
+	loff_t actread;
+	struct ext2fs_node fdiro;
+	int len;
+	struct ext2_dirent dirent;
+
+	*dentp = NULL;
+
+	ret = ext4fs_find_file(dirs->dirname, &ext4fs_root->diropen, &dir,
+			       FILETYPE_DIRECTORY);
+	if (ret != 1) {
+		ret = -ENOENT;
+		goto out;
+	}
+	if (!dir->inode_read) {
+		ret = ext4fs_read_inode(dir->data, dir->ino, &dir->inode);
+		if (!ret) {
+			ret = -EIO;
+			goto out;
+		}
+	}
+
+	if (dirs->fpos >= le32_to_cpu(dir->inode.size))
+		return -ENOENT;
+
+	memset(dent, 0, sizeof(struct fs_dirent));
+
+	while (dirs->fpos < le32_to_cpu(dir->inode.size)) {
+		ret = ext4fs_read_file(dir, dirs->fpos,
+				       sizeof(struct ext2_dirent),
+				       (char *)&dirent, &actread);
+		if (ret < 0)
+			return -ret;
+
+		if (!dirent.direntlen)
+			return -EIO;
+
+		if (dirent.namelen)
+			break;
+
+		dirs->fpos += le16_to_cpu(dirent.direntlen);
+	}
+
+	len = min(FS_DIRENT_NAME_LEN - 1, (int)dirent.namelen);
+
+	ret = ext4fs_read_file(dir, dirs->fpos + sizeof(struct ext2_dirent),
+			       len, dent->name, &actread);
+	if (ret < 0)
+		goto out;
+	dent->name[len] = '\0';
+
+	fdiro.data = dir->data;
+	fdiro.ino = le32_to_cpu(dirent.inode);
+
+	ret = ext4fs_read_inode(dir->data, fdiro.ino, &fdiro.inode);
+	if (!ret) {
+		ret = -EIO;
+		goto out;
+	}
+
+	switch (le16_to_cpu(fdiro.inode.mode) & FILETYPE_INO_MASK) {
+	case FILETYPE_INO_DIRECTORY:
+		dent->type = FS_DT_DIR;
+		break;
+	case FILETYPE_INO_SYMLINK:
+		dent->type = FS_DT_LNK;
+		break;
+	case FILETYPE_INO_REG:
+		dent->type = FS_DT_REG;
+		break;
+	default:
+		dent->type = FILETYPE_UNKNOWN;
+	}
+
+	rtc_to_tm(fdiro.inode.atime, &dent->access_time);
+	rtc_to_tm(fdiro.inode.ctime, &dent->create_time);
+	rtc_to_tm(fdiro.inode.mtime, &dent->change_time);
+
+	dirs->fpos += le16_to_cpu(dirent.direntlen);
+	dent->size = fdiro.inode.size;
+	*dentp = dent;
+	ret = 0;
+
+out:
+	if (dir)
+		ext4fs_free_node(dir, &ext4fs_root->diropen);
+
+	return ret;
+}
+
+void ext4fs_closedir(struct fs_dir_stream *fs_dirs)
+{
+	struct ext4_dir_stream *dirs = (struct ext4_dir_stream *)fs_dirs;
+
+	if (!dirs)
+		return;
+
+	free(dirs->dirname);
+	free(dirs);
 }
 
 int ext4fs_exists(const char *filename)
 {
 	struct ext2fs_node *dirnode = NULL;
 	int filetype;
+	int ret;
 
 	if (!filename)
 		return 0;
 
-	return ext4fs_find_file1(filename, &ext4fs_root->diropen, &dirnode,
+	ret = ext4fs_find_file1(filename, &ext4fs_root->diropen, &dirnode,
-				 &filetype);
+				&filetype);
+	if (dirnode)
+		ext4fs_free_node(dirnode, &ext4fs_root->diropen);
+
+	return ret;
 }
 
 int ext4fs_size(const char *filename, loff_t *size)

fs/fs.c

@@ -220,7 +220,7 @@ static struct fstype_info fstypes[] = {
 		.null_dev_desc_ok = false,
 		.probe = ext4fs_probe,
 		.close = ext4fs_close,
-		.ls = ext4fs_ls,
+		.ls = fs_ls_generic,
 		.exists = ext4fs_exists,
 		.size = ext4fs_size,
 		.read = ext4_read_file,
@@ -232,7 +232,9 @@ static struct fstype_info fstypes[] = {
 		.ln = fs_ln_unsupported,
 #endif
 		.uuid = ext4fs_uuid,
-		.opendir = fs_opendir_unsupported,
+		.opendir = ext4fs_opendir,
+		.readdir = ext4fs_readdir,
+		.closedir = ext4fs_closedir,
 		.unlink = fs_unlink_unsupported,
 		.mkdir = fs_mkdir_unsupported,
 	},

include/ext4fs.h

@@ -27,6 +27,7 @@
 #ifndef __EXT4__
 #define __EXT4__
 #include <ext_common.h>
+#include <fs.h>
 
 struct disk_partition;
 
@@ -218,4 +219,7 @@ int ext4fs_uuid(char *uuid_str);
 void ext_cache_init(struct ext_block_cache *cache);
 void ext_cache_fini(struct ext_block_cache *cache);
 int ext_cache_read(struct ext_block_cache *cache, lbaint_t block, int size);
+int ext4fs_opendir(const char *dirname, struct fs_dir_stream **dirsp);
+int ext4fs_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
+void ext4fs_closedir(struct fs_dir_stream *dirs);
 #endif

lib/efi_loader/efi_file.c

@@ -864,8 +864,16 @@ static efi_status_t EFIAPI efi_file_getinfo(struct efi_file_handle *file,
 		}
 
 		ret = efi_get_file_size(fh, &file_size);
-		if (ret != EFI_SUCCESS)
+		if (ret != EFI_SUCCESS) {
-			goto error;
+			if (!fh->isdir)
+				goto error;
+			/*
+			 * Some file drivers don't implement fs_size() for
+			 * directories. Use a dummy non-zero value.
+			 */
+			file_size = 4096;
+			ret = EFI_SUCCESS;
+		}
 
 		memset(info, 0, required_size);
 
@@ -976,14 +984,16 @@ static efi_status_t EFIAPI efi_file_setinfo(struct efi_file_handle *file,
 		}
 		free(new_file_name);
 		/* Check for truncation */
-		ret = efi_get_file_size(fh, &file_size);
+		if (!fh->isdir) {
-		if (ret != EFI_SUCCESS)
+			ret = efi_get_file_size(fh, &file_size);
-			goto out;
+			if (ret != EFI_SUCCESS)
-		if (file_size != info->file_size) {
+				goto out;
-			/* TODO: we do not support truncation */
+			if (file_size != info->file_size) {
-			EFI_PRINT("Truncation not supported\n");
+				/* TODO: we do not support truncation */
-			ret = EFI_ACCESS_DENIED;
+				EFI_PRINT("Truncation not supported\n");
-			goto out;
+				ret = EFI_ACCESS_DENIED;
+				goto out;
+			}
 		}
 		/*
 		 * We do not care for the other attributes

test/py/tests/test_env.py

@@ -488,7 +488,7 @@ def test_env_ext4(state_test_env):
         assert 'Loading Environment from EXT4... OK' in response
 
         response = c.run_command('ext4ls host 0:0')
-        assert '8192 uboot.env' in response
+        assert '8192   uboot.env' in response
 
         response = c.run_command('env info')
         assert 'env_valid = valid' in response

test/py/tests/test_fs/test_basic.py

@@ -33,10 +33,7 @@ class TestFsBasic(object):
             # In addition, test with a nonexistent directory to see if we crash.
             output = u_boot_console.run_command(
                 '%sls host 0:0 invalid_d' % fs_type)
-            if fs_type == 'ext4':
+            assert('' == output)
-                assert('Can not find directory' in output)
-            else:
-                assert('' == output)
 
     def test_fs2(self, u_boot_console, fs_obj_basic):
         """