2023-04-18 12:54:20

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [PATCH] gfs2: Move a variable assignment behind a null pointer check in inode_go_dump()

Hi Markus,

On Thu, Apr 13, 2023 at 9:23 PM Markus Elfring <[email protected]> wrote:
> Date: Thu, 13 Apr 2023 20:54:30 +0200
>
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “inode_go_dump”.
>
> Thus avoid the risk for undefined behaviour by moving the assignment
> for the variable “inode” behind the null pointer check.
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: 27a2660f1ef944724956d92e8a312b6da0936fae ("gfs2: Dump nrpages for inodes and their glocks")

Okay, that's a worthwhile cleanup. It doesn't actually fix a bug, so
I'm not going to add a Fixes tag, though.

> Signed-off-by: Markus Elfring <[email protected]>
> ---
> fs/gfs2/glops.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index b65950e76be5..6e33c8058059 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -535,12 +535,13 @@ static void inode_go_dump(struct seq_file *seq, struct gfs2_glock *gl,
> const char *fs_id_buf)
> {
> struct gfs2_inode *ip = gl->gl_object;
> - struct inode *inode = &ip->i_inode;
> + struct inode *inode;
> unsigned long nrpages;
>
> if (ip == NULL)
> return;
>
> + inode = &ip->i_inode;
> xa_lock_irq(&inode->i_data.i_pages);
> nrpages = inode->i_data.nrpages;
> xa_unlock_irq(&inode->i_data.i_pages);
> --
> 2.40.0
>

Thanks,
Andreas