[Cbe-oss-dev] [PATCH] memory leak on SPU coredump

[Akinobu Mita]

On Tue, Mar 13, 2007 at 12:40:14PM +0100, Arnd Bergmann wrote:
> > Index: 2.6-git-ps3/arch/powerpc/platforms/cell/spufs/coredump.c
> > ===================================================================
> > --- 2.6-git-ps3.orig/arch/powerpc/platforms/cell/spufs/coredump.c
> > +++ 2.6-git-ps3/arch/powerpc/platforms/cell/spufs/coredump.c
> > @@ -212,6 +212,7 @@ static void spufs_arch_write_note(struct
> >                 }
> >         } while (rc == bufsz && total < sz);
> >  
> > +       kfree(buf);
> >         spufs_dump_seek(file, roundup((unsigned long)file->f_pos
> >                                                 - total + sz, 4));
> >  }
> 
> You add the kfree only in one exit path, not in the case where the
> function is left after an error condition.
> It probably also makes sense to convert this to get_zeroed_page at
> the same time. Please try the patch below (I haven't tested it myself)
> and see if it looks good to you

There is one more exit path which is not covered by your patch.

        ctx = ctx_info->ctx;
        if (!ctx) {
                return;
        }

Otherwise, your patch looks good to me.

> 	Arnd <><
> 
> diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
> index 725e195..99fa791 100644
> --- a/arch/powerpc/platforms/cell/spufs/coredump.c
> +++ b/arch/powerpc/platforms/cell/spufs/coredump.c
> @@ -169,12 +169,12 @@ static void spufs_arch_write_note(struct spufs_ctx_info *ctx_info, int i,
>  	struct spu_context *ctx;
>  	loff_t pos = 0;
>  	int sz, dfd, rc, total = 0;
> -	const int bufsz = 4096;
> +	const int bufsz = PAGE_SIZE;
>  	char *name;
>  	char fullname[80], *buf;
>  	struct elf_note en;
>  
> -	buf = kmalloc(bufsz, GFP_KERNEL);
> +	buf = (void *)get_zeroed_page(GFP_KERNEL);
>  	if (!buf)
>  		return;
>  
> @@ -197,23 +197,25 @@ static void spufs_arch_write_note(struct spufs_ctx_info *ctx_info, int i,
>  	en.n_type = NT_SPU;
>  
>  	if (!spufs_dump_write(file, &en, sizeof(en)))
> -		return;
> +		goto out;
>  	if (!spufs_dump_write(file, fullname, en.n_namesz))
> -		return;
> +		goto out;
>  	if (!spufs_dump_seek(file, roundup((unsigned long)file->f_pos, 4)))
> -		return;
> +		goto out;
>  
>  	do {
>  		rc = do_coredump_read(i, ctx, buf, bufsz, &pos);
>  		if (rc > 0) {
>  			if (!spufs_dump_write(file, buf, rc))
> -				return;
> +				goto out;
>  			total += rc;
>  		}
>  	} while (rc == bufsz && total < sz);
>  
>  	spufs_dump_seek(file, roundup((unsigned long)file->f_pos
>  						- total + sz, 4));
> +out:
> +	free_page((unsigned long)buf);
>  }
>  
>  static void spufs_arch_write_notes(struct file *file)
>