[Cluster-devel] [PATCH] libgfs2: Add sd_heightsize bounds checking in read_sb

[Steven Whitehouse]

Hi,

Looks good. thanks,

Steve.

On Mon, 2013-10-28 at 16:56 +0000, Andrew Price wrote:
> read_sb wasn't checking that x was less than the size of sd_heightsize
> when looping over it. This patch adds a check for that. This resolves a
> segfault in all tools which use read_sb, when the sb_bsize has been
> zeroed in the superblock.
> 
> A test case has been added for this scenario in tests/fsck.at
> 
> Resolves: bz#1019226
> 
> Signed-off-by: Andrew Price <anprice at redhat.com>
> ---
>  gfs2/libgfs2/super.c | 2 +-
>  tests/fsck.at        | 8 ++++++++
>  tests/testsuite.at   | 9 +++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
>  create mode 100644 tests/fsck.at
> 
> diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
> index f87734a..8ffd144 100644
> --- a/gfs2/libgfs2/super.c
> +++ b/gfs2/libgfs2/super.c
> @@ -88,7 +88,7 @@ int read_sb(struct gfs2_sbd *sdp)
>  	sdp->sd_hash_ptrs = sdp->sd_hash_bsize / sizeof(uint64_t);
>  	sdp->sd_heightsize[0] = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode);
>  	sdp->sd_heightsize[1] = sdp->sd_sb.sb_bsize * sdp->sd_diptrs;
> -	for (x = 2; ; x++){
> +	for (x = 2; x <= GFS2_MAX_META_HEIGHT; x++){
>  		space = sdp->sd_heightsize[x - 1] * sdp->sd_inptrs;
>  		/* FIXME: Do we really need this first check?? */
>  		if (space / sdp->sd_inptrs != sdp->sd_heightsize[x - 1] ||
> diff --git a/tests/fsck.at b/tests/fsck.at
> new file mode 100644
> index 0000000..34c5bd5
> --- /dev/null
> +++ b/tests/fsck.at
> @@ -0,0 +1,8 @@
> +AT_TESTED([fsck.gfs2])
> +AT_BANNER([fsck.gfs2 tests])
> +
> +AT_SETUP([Zeroed block size])
> +GFS_LANG_CHECK(
> +	[mkfs.gfs2 -O -p lock_nolock $GFS_TGT],
> +	[set sb { sb_bsize: 0 }])
> +AT_CLEANUP
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index 2c74985..fc90879 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -9,8 +9,17 @@ m4_define([GFS_FSCK_CHECK],
>  AT_CHECK($1, 0, [ignore], [ignore])
>  AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore])])
>  
> +# Regenerate, mkfs, modify fs with gfs2l, fsck
> +m4_define([GFS_LANG_CHECK],
> +[GFS_TGT_REGEN
> +AT_CHECK($1, 0, [ignore], [ignore])
> +AT_CHECK([echo "$2" | gfs2l ${GFS_TGT}], 0, [ignore], [ignore])
> +AT_CHECK([fsck.gfs2 -y $GFS_TGT], 1, [ignore], [ignore])
> +AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore])])
> +
>  AT_INIT([])
>  AT_COLOR_TESTS
>  
>  m4_include([mkfs.at])
> +m4_include([fsck.at])
>  m4_include([libgfs2.at])