[Cluster-devel] [PATCH] hexedit: avoid NULL dereference upon failed malloc

[Andrew Price <andy@andrewprice.me.uk>]

Hi,

This patch gives a build error:

hexedit.c:1500: error: incompatible type for argument 1 of ?memset?

On 17/06/09 17:00, Jim Meyering wrote:
> If the malloc of more_indir fails, the subsequent deref
> via memset would cause a segfault.
> 
> I chose to avoid that by making more_indir a stack-local.
> If it's 512-byte size is too big for your stack
> requirements, let me know and I'll rewrite to
> use malloc -- though doing it that way (and taking
> care to avoid leaks), would probably end up uglier.
> 
>>From fda0f39b0389088b0ea66216aed21d4f5f1bb604 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@redhat.com>
> Date: Wed, 17 Jun 2009 16:54:03 +0200
> Subject: [PATCH] hexedit: avoid NULL dereference upon failed malloc
> 
> * gfs2/edit/hexedit.c (display_indirect): Avoid unchecked malloc
> by declaring more_indir on the stack.
> ---
>  gfs2/edit/hexedit.c |   12 +++++-------
>  1 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
> index e8c6030..84b4be4 100644
> --- a/gfs2/edit/hexedit.c
> +++ b/gfs2/edit/hexedit.c
> @@ -1477,13 +1477,12 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
>  			file_offset = 0;
>  		if (!termlines && ((level + 1 < di.di_height) ||
>  				   (S_ISDIR(di.di_mode) && !level))) {
> -			struct iinfo *more_indir;
>  			int more_ind;
>  			char *tmpbuf;
>  			
> -			more_indir = malloc(sizeof(struct iinfo));
>  			tmpbuf = malloc(sbd.bsize);
>  			if (tmpbuf) {
> +				struct iinfo more_indir;
>  				lseek(sbd.device_fd,
>  				      ind->ii[pndx].block * sbd.bsize,
>  				      SEEK_SET);
> @@ -1500,18 +1499,17 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
>  				}
>  				memset(more_indir, 0, sizeof(struct iinfo));
>  				if (S_ISDIR(di.di_mode)) {
> -					do_leaf_extended(tmpbuf, more_indir);
> -					display_leaf(more_indir);
> +					do_leaf_extended(tmpbuf, &more_indir);
> +					display_leaf(&more_indir);
>  				} else {
>  					more_ind = do_indirect_extended(tmpbuf,
> -									more_indir);
> -					display_indirect(more_indir,
> +									&more_indir);
> +					display_indirect(&more_indir,
>  							 more_ind, level + 1,
>  							 file_offset);
>  				}
>  				free(tmpbuf);
>  			}
> -			free(more_indir);
>  		}
>  		print_entry_ndx = pndx; /* restore after recursion */
>  		eol(0);