[Cluster-devel] [libgfs2 PATCH] libgfs2: Check block range when inserting into rgrp tree

[Cluster-devel] [libgfs2 PATCH] libgfs2: Check block range when inserting into rgrp tree

[Bob Peterson]

Hi,

This patch adds checks to function rindex_read to make sure the
rgrp starting address isn't grossly outside the file system.
It may be in the case of severely corrupt file systems from fsck.
If we added them to the rgrp tree, our calculations will get
screwed up, eventually causing a segfault.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
index b956366..73354ff 100644
--- a/gfs2/libgfs2/super.c
+++ b/gfs2/libgfs2/super.c
@@ -166,6 +166,12 @@ int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane)
 			return -1;
 
 		gfs2_rindex_in(&ri, (char *)&buf.bufgfs2);
+		if (gfs2_check_range(sdp, ri.ri_addr) != 0) {
+			*sane = 0;
+			if (prev_rgd == NULL)
+				return -1;
+			ri.ri_addr = prev_rgd->ri.ri_addr + prev_rgd->length;
+		}
 		rgd = rgrp_insert(&sdp->rgtree, ri.ri_addr);
 		memcpy(&rgd->ri, &ri, sizeof(struct gfs2_rindex));
 

[Cluster-devel] [libgfs2 PATCH] libgfs2: Check block range when inserting into rgrp tree

[Andrew Price]

On 14/09/15 18:19, Bob Peterson wrote:
> Hi,
>
> This patch adds checks to function rindex_read to make sure the
> rgrp starting address isn't grossly outside the file system.
> It may be in the case of severely corrupt file systems from fsck.
> If we added them to the rgrp tree, our calculations will get
> screwed up, eventually causing a segfault.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
> index b956366..73354ff 100644
> --- a/gfs2/libgfs2/super.c
> +++ b/gfs2/libgfs2/super.c
> @@ -166,6 +166,12 @@ int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane)
>   			return -1;
>
>   		gfs2_rindex_in(&ri, (char *)&buf.bufgfs2);
> +		if (gfs2_check_range(sdp, ri.ri_addr) != 0) {

I guess since we don't have the full rindex yet to get the fs size it's 
checking against the device size here. Assuming other tools using this 
function have initialized sdp->fssize sensibly before calling it: ACK.

Andy

> +			*sane = 0;
> +			if (prev_rgd == NULL)
> +				return -1;
> +			ri.ri_addr = prev_rgd->ri.ri_addr + prev_rgd->length;
> +		}
>   		rgd = rgrp_insert(&sdp->rgtree, ri.ri_addr);
>   		memcpy(&rgd->ri, &ri, sizeof(struct gfs2_rindex));
>
>