Re: [PATCH] fs: ext4fs: Free memory while handling errors

[Quentin Schulz]

Hi Francois,

On 12/9/25 9:59 AM, Francois Berder wrote:
If zalloc fails, one needs to free memory previously
allocated in the function. This commit makes sure that
we do not leak any memory.
Signed-off-by: Francois Berder <fber...@outlook.fr>
---
  fs/ext4/ext4_common.c  | 8 +++++++-
  fs/ext4/ext4_journal.c | 4 +++-
  fs/ext4/ext4_write.c   | 2 ++
  3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 8e6531fa3f0..f48a8ca48d3 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -727,8 +727,14 @@ static int parse_path(char **arr, char *dirname)
        /* add each path entry after root */
        while (token != NULL) {
                arr[i] = zalloc(strlen(token) + 1);
-               if (!arr[i])
+               if (!arr[i]) {
+                       int j;
+
+                       for (j = 0; j < i; j++)
+                               free(arr[j]);
+

Simply reuse i?

for (--i; i >= 0; i--)
    free(arr[i]);

? (i being an int makes i >= 0 safe in the loop condition (as in "won't 
happen") wrt underflow)

                        return -ENOMEM;
+               }
                memcpy(arr[i++], token, strlen(token));
                token = strtok(NULL, "/");
        }
diff --git a/fs/ext4/ext4_journal.c b/fs/ext4/ext4_journal.c
index 868a2c1804a..3a2e9f30c12 100644
--- a/fs/ext4/ext4_journal.c
+++ b/fs/ext4/ext4_journal.c
@@ -256,8 +256,10 @@ void ext4fs_push_revoke_blk(char *buffer)
        }
  
  	node->content = zalloc(fs->blksz);
-       if (node->content == NULL)
+       if (!node->content) {
+               free(node);
                return;
+       }
        memcpy(node->content, buffer, fs->blksz);
  
  	if (first_node == true) {
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 5b290f0d80d..20d4e47fd38 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -204,6 +204,7 @@ static void delete_double_indirect_block(struct ext2_inode 
*inode)
        if (inode->b.blocks.double_indir_block != 0) {
                di_buffer = zalloc(fs->blksz);
                if (!di_buffer) {
+                       free(journal_buffer);
                        printf("No memory\n");

What about just doing

goto fail;

here? As it'll free both dib_start_addr and journal_buffer (and that it 
seems free(x) shall check for x == NULL before doing anything).

                        return;
                }
@@ -303,6 +304,7 @@ static void delete_triple_indirect_block(struct ext2_inode 
*inode)
        if (inode->b.blocks.triple_indir_block != 0) {
                tigp_buffer = zalloc(fs->blksz);
                if (!tigp_buffer) {
+                       free(journal_buffer);
                        printf("No memory\n");

Ditto.

Cheers,
Quentin