[Cluster-devel] [PATCH] gfs2: fix possible reference leak in gfs2_check_blk_type

[Bob Peterson <rpeterso@redhat.com>]

----- Original Message -----
> In the fail path of gfs2_check_blk_type, forgetting to call
> gfs2_glock_dq_uninit will result in rgd_gh reference leak.
> 
> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
> ---
>  fs/gfs2/rgrp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index ee491bb9c1cc..dd3e888994f7 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -2526,8 +2526,10 @@ int gfs2_check_blk_type(struct gfs2_sbd *sdp, u64
> no_addr, unsigned int type)
>  
>  	rbm.rgd = rgd;
>  	error = gfs2_rbm_from_block(&rbm, no_addr);
> -	if (WARN_ON_ONCE(error))
> +	if (WARN_ON_ONCE(error)) {
> +		gfs2_glock_dq_uninit(&rgd_gh);
>  		goto fail;
> +	}
>  
>  	if (gfs2_testbit(&rbm, false) != type)
>  		error = -ESTALE;
> --
> 2.17.1

Hi,

Thanks. This is clearly a bug. Can we do something like this instead?
Andreas, which do you prefer?

 fs/gfs2/rgrp.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 92d799a193b8..b26d6fcb6915 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -2529,12 +2529,10 @@ int gfs2_check_blk_type(struct gfs2_sbd *sdp, u64 no_addr, unsigned int type)
 
 	rbm.rgd = rgd;
 	error = gfs2_rbm_from_block(&rbm, no_addr);
-	if (WARN_ON_ONCE(error))
-		goto fail;
-
-	if (gfs2_testbit(&rbm, false) != type)
-		error = -ESTALE;
-
+	if (!WARN_ON_ONCE(error)) {
+		if (gfs2_testbit(&rbm, false) != type)
+			error = -ESTALE;
+	}
 	gfs2_glock_dq_uninit(&rgd_gh);
 fail:
 	return error;

Regards,

Bob Peterson