grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 1/2] fs/zfs/zfs.c: make_mdn() - avoid pointer downcasting


From: Jagannathan Raman
Subject: [PATCH 1/2] fs/zfs/zfs.c: make_mdn() - avoid pointer downcasting
Date: Thu, 2 Jun 2022 15:18:26 +0000

Coverity reports that the while loop in the following function uses
tainted data as boundary:
  fill_fs_info() -> dnode_get() -> zfs_log2()

The tainted originated from:
  fill_fs_info() -> make_mdn()

The defect type is "Untrusted loop bound" caused as a result of
"tainted_data_downcast". Coverity does not like the pointer downcast
here and we need to address it.

We believe Coverity flags pointer downcast for the following two
reasons:
1. External data: The pointer downcast could indicate that the source is
  external data, which we need to further sanitize - such as verifying its
  limits. In this case, the data is read from an external source, which is
  a disk. But, zio_read(), which reads the data from the disk, sanitizes it
  using a checksum. checksum is the best facility that ZFS offers to verify
  external data, and we don't believe a better way exists. Therefore, no
  further action is possible for this.

2. Corruption due to alignment: downcasting a pointer from a strict type
  to less strict type could result in data corruption. For example, the
  following cast would corrupt because uint32_t is 4-byte aligned, and
  won't be able to point to 0x1003 which is not 4-byte aligned.
    uint8_t *ptr = 0x1003;
    uint32_t *word = ptr; (incorrect, alignment issues)

  This patch converts the "osp" pointer in make_mdn() from a "void" type
  to "objset_phys_t" type to address this issue.

We are not sure if there are any other reasons why Coverity flags the
downcast. However, the fix for alignment issue masks/suppresses any
other issues from showing up.

Fixes: CID 314020

Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
---
 grub-core/fs/zfs/zfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 63b82abf3..d9c79663b 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -3156,7 +3156,7 @@ get_filesystem_dnode (dnode_end_t * mosmdn, char *fsname,
 static grub_err_t
 make_mdn (dnode_end_t * mdn, struct grub_zfs_data *data)
 {
-  void *osp;
+  objset_phys_t *osp;
   blkptr_t *bp;
   grub_size_t ospsize = 0;
   grub_err_t err;
@@ -3164,7 +3164,7 @@ make_mdn (dnode_end_t * mdn, struct grub_zfs_data *data)
   grub_dprintf ("zfs", "endian = %d\n", mdn->endian);
 
   bp = &(((dsl_dataset_phys_t *) DN_BONUS (&mdn->dn))->ds_bp);
-  err = zio_read (bp, mdn->endian, &osp, &ospsize, data);
+  err = zio_read (bp, mdn->endian, (void **) &osp, &ospsize, data);
   if (err)
     return err;
   if (ospsize < OBJSET_PHYS_SIZE_V14)
@@ -3175,7 +3175,7 @@ make_mdn (dnode_end_t * mdn, struct grub_zfs_data *data)
 
   mdn->endian = (grub_zfs_to_cpu64 (bp->blk_prop, mdn->endian)>>63) & 1;
   grub_memmove ((char *) &(mdn->dn),
-               (char *) &((objset_phys_t *) osp)->os_meta_dnode, DNODE_SIZE);
+               (char *) &(osp)->os_meta_dnode, DNODE_SIZE);
   grub_free (osp);
   return GRUB_ERR_NONE;
 }
-- 
2.31.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]