Re: [PATCH] fill_fs_info: pass pointer to dnode_end_t instead of value

[Darren Kenny]

Hi Jag,

These changes look good to me.

Just to confirm, you have run 'make check' and this has no negative
impact on the zfs tests?

Assuming that is the case...

On Friday, 2022-08-19 at 10:57:22 -04, Jagannathan Raman wrote:
> Coverity reports that dnode_end_t argument of fill_fs_info() is too
> large to pass-by-value. Therefore, replace the argument with a pointer.
>
> Fixes: CID 73631
>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  grub-core/fs/zfs/zfs.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
> index ffa0e5863..975c67242 100644
> --- a/grub-core/fs/zfs/zfs.c
> +++ b/grub-core/fs/zfs/zfs.c
> @@ -3983,14 +3983,23 @@ grub_zfs_getmdnobj (grub_device_t dev, const char 
> *fsfilename,
>    return err;
>  }
>  
> +/*
> + * Note: fill_fs_info() uses functions such as make_mdn() that modify
> + * the input dnode_end_t parameter. However, we should not allow it.
> + * Therefore, we are making mdn_in constant - fill_fs_info() makes a
> + * local copy of it.
> + */
>  static grub_err_t
>  fill_fs_info (struct grub_dirhook_info *info,
> -           dnode_end_t mdn, struct grub_zfs_data *data)
> +           const dnode_end_t *mdn_in, struct grub_zfs_data *data)
>  {
>    grub_err_t err;
>    dnode_end_t dn;
>    grub_uint64_t objnum;
>    grub_uint64_t headobj;
> +  dnode_end_t mdn;
> +
> +  grub_memcpy (&mdn, mdn_in, sizeof (*mdn_in));
>  
>    grub_memset (info, 0, sizeof (*info));
>  
> @@ -4148,7 +4157,7 @@ iterate_zap_fs (const char *name, grub_uint64_t val,
>    if (mdn.dn.dn_type != DMU_OT_DSL_DIR)
>      return 0;
>  
> -  err = fill_fs_info (&info, mdn, ctx->data);
> +  err = fill_fs_info (&info, &mdn, ctx->data);
>    if (err)
>      {
>        grub_errno = 0;
> @@ -4179,7 +4188,7 @@ iterate_zap_snap (const char *name, grub_uint64_t val,
>    if (mdn.dn.dn_type != DMU_OT_DSL_DATASET)
>      return 0;
>  
> -  err = fill_fs_info (&info, mdn, ctx->data);
> +  err = fill_fs_info (&info, &mdn, ctx->data);
>    if (err)
>      {
>        grub_errno = 0;
> @@ -4224,7 +4233,7 @@ grub_zfs_dir (grub_device_t device, const char *path,
>        dnode_end_t dn;
>        struct grub_dirhook_info info;
>  
> -      err = fill_fs_info (&info, data->dnode, data);
> +      err = fill_fs_info (&info, &data->dnode, data);
>        if (err)
>       {
>         zfs_unmount (data);
> -- 
> 2.20.1