|
|
468bd4 |
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
468bd4 |
From: Paulo Flabiano Smorigo <pfsmorigo@canonical.com>
|
|
|
468bd4 |
Date: Mon, 14 Dec 2020 18:54:49 -0300
|
|
|
468bd4 |
Subject: [PATCH] zfs: Fix resource leaks while constructing path
|
|
|
468bd4 |
|
|
|
468bd4 |
There are several exit points in dnode_get_path() that are causing possible
|
|
|
468bd4 |
memory leaks.
|
|
|
468bd4 |
|
|
|
468bd4 |
In the while(1) the correct exit mechanism should not be to do a direct return,
|
|
|
468bd4 |
but to instead break out of the loop, setting err first if it is not already set.
|
|
|
468bd4 |
|
|
|
468bd4 |
The reason behind this is that the dnode_path is a linked list, and while doing
|
|
|
468bd4 |
through this loop, it is being allocated and built up - the only way to
|
|
|
468bd4 |
correctly unravel it is to traverse it, which is what is being done at the end
|
|
|
468bd4 |
of the function outside of the loop.
|
|
|
468bd4 |
|
|
|
468bd4 |
Several of the existing exit points correctly did a break, but not all so this
|
|
|
468bd4 |
change makes that more consistent and should resolve the leaking of memory as
|
|
|
468bd4 |
found by Coverity.
|
|
|
468bd4 |
|
|
|
468bd4 |
Fixes: CID 73741
|
|
|
468bd4 |
|
|
|
468bd4 |
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@canonical.com>
|
|
|
468bd4 |
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
|
|
|
468bd4 |
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
|
|
|
468bd4 |
---
|
|
|
468bd4 |
grub-core/fs/zfs/zfs.c | 30 +++++++++++++++++++++---------
|
|
|
468bd4 |
1 file changed, 21 insertions(+), 9 deletions(-)
|
|
|
468bd4 |
|
|
|
468bd4 |
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
|
|
|
f6e916 |
index 3dfde0807..44d8bde6b 100644
|
|
|
468bd4 |
--- a/grub-core/fs/zfs/zfs.c
|
|
|
468bd4 |
+++ b/grub-core/fs/zfs/zfs.c
|
|
|
468bd4 |
@@ -2836,8 +2836,8 @@ dnode_get_path (struct subvolume *subvol, const char *path_in, dnode_end_t *dn,
|
|
|
468bd4 |
|
|
|
468bd4 |
if (dnode_path->dn.dn.dn_type != DMU_OT_DIRECTORY_CONTENTS)
|
|
|
468bd4 |
{
|
|
|
468bd4 |
- grub_free (path_buf);
|
|
|
468bd4 |
- return grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("not a directory"));
|
|
|
468bd4 |
+ err = grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("not a directory"));
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
}
|
|
|
468bd4 |
err = zap_lookup (&(dnode_path->dn), cname, &objnum,
|
|
|
468bd4 |
data, subvol->case_insensitive);
|
|
|
468bd4 |
@@ -2879,11 +2879,18 @@ dnode_get_path (struct subvolume *subvol, const char *path_in, dnode_end_t *dn,
|
|
|
468bd4 |
<< SPA_MINBLOCKSHIFT);
|
|
|
468bd4 |
|
|
|
468bd4 |
if (blksz == 0)
|
|
|
468bd4 |
- return grub_error(GRUB_ERR_BAD_FS, "0-sized block");
|
|
|
468bd4 |
+ {
|
|
|
468bd4 |
+ err = grub_error (GRUB_ERR_BAD_FS, "0-sized block");
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
+ }
|
|
|
468bd4 |
|
|
|
468bd4 |
sym_value = grub_malloc (sym_sz);
|
|
|
468bd4 |
if (!sym_value)
|
|
|
468bd4 |
- return grub_errno;
|
|
|
468bd4 |
+ {
|
|
|
468bd4 |
+ err = grub_errno;
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
+ }
|
|
|
468bd4 |
+
|
|
|
468bd4 |
for (block = 0; block < (sym_sz + blksz - 1) / blksz; block++)
|
|
|
468bd4 |
{
|
|
|
468bd4 |
void *t;
|
|
|
468bd4 |
@@ -2893,7 +2900,7 @@ dnode_get_path (struct subvolume *subvol, const char *path_in, dnode_end_t *dn,
|
|
|
468bd4 |
if (err)
|
|
|
468bd4 |
{
|
|
|
468bd4 |
grub_free (sym_value);
|
|
|
468bd4 |
- return err;
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
}
|
|
|
468bd4 |
|
|
|
468bd4 |
movesize = sym_sz - block * blksz;
|
|
|
468bd4 |
@@ -2903,6 +2910,8 @@ dnode_get_path (struct subvolume *subvol, const char *path_in, dnode_end_t *dn,
|
|
|
468bd4 |
grub_memcpy (sym_value + block * blksz, t, movesize);
|
|
|
468bd4 |
grub_free (t);
|
|
|
468bd4 |
}
|
|
|
468bd4 |
+ if (err)
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
free_symval = 1;
|
|
|
468bd4 |
}
|
|
|
468bd4 |
path = path_buf = grub_malloc (sym_sz + grub_strlen (oldpath) + 1);
|
|
|
468bd4 |
@@ -2911,7 +2920,8 @@ dnode_get_path (struct subvolume *subvol, const char *path_in, dnode_end_t *dn,
|
|
|
468bd4 |
grub_free (oldpathbuf);
|
|
|
468bd4 |
if (free_symval)
|
|
|
468bd4 |
grub_free (sym_value);
|
|
|
468bd4 |
- return grub_errno;
|
|
|
468bd4 |
+ err = grub_errno;
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
}
|
|
|
468bd4 |
grub_memcpy (path, sym_value, sym_sz);
|
|
|
468bd4 |
if (free_symval)
|
|
|
468bd4 |
@@ -2949,11 +2959,12 @@ dnode_get_path (struct subvolume *subvol, const char *path_in, dnode_end_t *dn,
|
|
|
468bd4 |
|
|
|
468bd4 |
err = zio_read (bp, dnode_path->dn.endian, &sahdrp, NULL, data);
|
|
|
468bd4 |
if (err)
|
|
|
468bd4 |
- return err;
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
}
|
|
|
468bd4 |
else
|
|
|
468bd4 |
{
|
|
|
468bd4 |
- return grub_error (GRUB_ERR_BAD_FS, "filesystem is corrupt");
|
|
|
468bd4 |
+ err = grub_error (GRUB_ERR_BAD_FS, "filesystem is corrupt");
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
}
|
|
|
468bd4 |
|
|
|
468bd4 |
hdrsize = SA_HDR_SIZE (((sa_hdr_phys_t *) sahdrp));
|
|
|
468bd4 |
@@ -2974,7 +2985,8 @@ dnode_get_path (struct subvolume *subvol, const char *path_in, dnode_end_t *dn,
|
|
|
468bd4 |
if (!path_buf)
|
|
|
468bd4 |
{
|
|
|
468bd4 |
grub_free (oldpathbuf);
|
|
|
468bd4 |
- return grub_errno;
|
|
|
468bd4 |
+ err = grub_errno;
|
|
|
468bd4 |
+ break;
|
|
|
468bd4 |
}
|
|
|
468bd4 |
grub_memcpy (path, sym_value, sym_sz);
|
|
|
468bd4 |
path [sym_sz] = 0;
|