[Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Prevent rgrp segment overflow

[Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Prevent rgrp segment overflow

[Bob Peterson]

Hi,

This is the first of 3 patches for problems I found testing rg repair.

Patch 0103dc0 (fsck.gfs2: Detect multiple rgrp grow segments) allowed
fsck to detect multiple gfs2_grow segments that may have placed
resource groups on unexpected boundaries. However, it failed to check
or take appropriate action when too many segments were found.
Instead, it would write past the arrays and corrupt memory. This
patch adds some sanity checks to prevent this from happening.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 55057ce..7479458 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -164,6 +164,8 @@ static int find_shortest_rgdist(struct gfs2_sbd *sdp, uint64_t *dist_array,
 				/* That last one didn't pan out, so: */
 				dist_cnt[gsegment]--;
 				gsegment++;
+				if (gsegment >= MAX_RGSEGMENTS)
+					break;
 			}
 			if ((blk - block_last_rg) > (524288 * 2)) {
 				log_info(_("No rgrps were found within 4GB "
@@ -217,6 +219,8 @@ static int find_shortest_rgdist(struct gfs2_sbd *sdp, uint64_t *dist_array,
 			}
 		} else {
 			gsegment++;
+			if (gsegment >= MAX_RGSEGMENTS)
+				break;
 		}
 		block_last_rg = blk;
 		if (rgs_sampled < 6)
@@ -224,7 +228,7 @@ static int find_shortest_rgdist(struct gfs2_sbd *sdp, uint64_t *dist_array,
 		else
 			blk += shortest_dist_btwn_rgs - 1;
 	}
-	if (gsegment > MAX_RGSEGMENTS) {
+	if (gsegment >= MAX_RGSEGMENTS) {
 		log_err(_("Maximum number of rgrp grow segments reached.\n"));
 		log_err(_("This file system cannot be repaired with fsck.\n"));
 		gsegment = 0;

[Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Prevent rgrp segment overflow

[Andrew Price]

On 13/01/17 20:28, Bob Peterson wrote:
> Hi,
>
> This is the first of 3 patches for problems I found testing rg repair.

ACK to all three.

Thanks,
Andy

> Patch 0103dc0 (fsck.gfs2: Detect multiple rgrp grow segments) allowed
> fsck to detect multiple gfs2_grow segments that may have placed
> resource groups on unexpected boundaries. However, it failed to check
> or take appropriate action when too many segments were found.
> Instead, it would write past the arrays and corrupt memory. This
> patch adds some sanity checks to prevent this from happening.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
> index 55057ce..7479458 100644
> --- a/gfs2/fsck/rgrepair.c
> +++ b/gfs2/fsck/rgrepair.c
> @@ -164,6 +164,8 @@ static int find_shortest_rgdist(struct gfs2_sbd *sdp, uint64_t *dist_array,
>  				/* That last one didn't pan out, so: */
>  				dist_cnt[gsegment]--;
>  				gsegment++;
> +				if (gsegment >= MAX_RGSEGMENTS)
> +					break;
>  			}
>  			if ((blk - block_last_rg) > (524288 * 2)) {
>  				log_info(_("No rgrps were found within 4GB "
> @@ -217,6 +219,8 @@ static int find_shortest_rgdist(struct gfs2_sbd *sdp, uint64_t *dist_array,
>  			}
>  		} else {
>  			gsegment++;
> +			if (gsegment >= MAX_RGSEGMENTS)
> +				break;
>  		}
>  		block_last_rg = blk;
>  		if (rgs_sampled < 6)
> @@ -224,7 +228,7 @@ static int find_shortest_rgdist(struct gfs2_sbd *sdp, uint64_t *dist_array,
>  		else
>  			blk += shortest_dist_btwn_rgs - 1;
>  	}
> -	if (gsegment > MAX_RGSEGMENTS) {
> +	if (gsegment >= MAX_RGSEGMENTS) {
>  		log_err(_("Maximum number of rgrp grow segments reached.\n"));
>  		log_err(_("This file system cannot be repaired with fsck.\n"));
>  		gsegment = 0;
>