All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kfree in fs/nfsd/nfs4xdr.c:savemem()
@ 2006-12-27 13:18 Benny Halevy
  2006-12-29 22:58 ` J. Bruce Fields
  0 siblings, 1 reply; 3+ messages in thread
From: Benny Halevy @ 2006-12-27 13:18 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: nfs, nfsv4

The wrong pointer is being kfree'd in savemem() when defer_free
returns with an error.

Benny

Signed-off-by: Benny Halevy <bhalevy@panasas.com>

diff -rp -U 11 a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
--- a/fs/nfsd/nfs4xdr.c 2006-12-18 19:18:45.000000000 +0200
+++ b/fs/nfsd/nfs4xdr.c 2006-12-25 09:15:58.000000000 +0200
@@ -197,23 +197,23 @@ static char *savemem(struct nfsd4_compou
        if (p == argp->tmp) {
                new = kmalloc(nbytes, GFP_KERNEL);
                if (!new) return NULL;
                p = new;
                memcpy(p, argp->tmp, nbytes);
        } else {
                if (p != argp->tmpp)
                        BUG();
                argp->tmpp = NULL;
        }
        if (defer_free(argp, kfree, p)) {
-               kfree(new);
+               kfree(p);
                return NULL;
        } else
                return (char *)p;
 }


 static int
 nfsd4_decode_bitmap(struct nfsd4_compoundargs *argp, u32 *bmval)
 {
        u32 bmlen;
        DECODE_HEAD;


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kfree in fs/nfsd/nfs4xdr.c:savemem()
  2006-12-27 13:18 [PATCH] kfree in fs/nfsd/nfs4xdr.c:savemem() Benny Halevy
@ 2006-12-29 22:58 ` J. Bruce Fields
  2006-12-31 20:22   ` Halevy, Benny
  0 siblings, 1 reply; 3+ messages in thread
From: J. Bruce Fields @ 2006-12-29 22:58 UTC (permalink / raw)
  To: Benny Halevy; +Cc: nfsv4, nfs, Trond Myklebust

On Wed, Dec 27, 2006 at 03:18:11PM +0200, Benny Halevy wrote:
> The wrong pointer is being kfree'd in savemem() when defer_free
> returns with an error.

Makes sense to me, thanks!

That makes "new" unnecessary.  Any objection to the following?
(untested).

Since this seems to be a rare memory leak, I'm inclined to leave this
till after 2.6.20.

--b.

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index dabd527..91284a0 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -199,18 +199,16 @@ defer_free(struct nfsd4_compoundargs *argp,
 
 static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
 {
-	void *new = NULL;
 	if (p == argp->tmp) {
-		new = kmalloc(nbytes, GFP_KERNEL);
-		if (!new) return NULL;
-		p = new;
+		p = kmalloc(nbytes, GFP_KERNEL);
+		if (!p) return NULL;
 		memcpy(p, argp->tmp, nbytes);
 	} else {
 		BUG_ON(p != argp->tmpp);
 		argp->tmpp = NULL;
 	}
 	if (defer_free(argp, kfree, p)) {
-		kfree(new);
+		kfree(p);
 		return NULL;
 	} else
 		return (char *)p;

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] kfree in fs/nfsd/nfs4xdr.c:savemem()
  2006-12-29 22:58 ` J. Bruce Fields
@ 2006-12-31 20:22   ` Halevy, Benny
  0 siblings, 0 replies; 3+ messages in thread
From: Halevy, Benny @ 2006-12-31 20:22 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: nfsv4, nfs, Trond Myklebust

J. Bruce Fields wrote:
>  
> On Wed, Dec 27, 2006 at 03:18:11PM +0200, Benny Halevy wrote:
> > The wrong pointer is being kfree'd in savemem() when defer_free
> > returns with an error.
> 
> Makes sense to me, thanks!
> 
> That makes "new" unnecessary.  Any objection to the following?
> (untested).

This seems to be right on the spot...

> 
> Since this seems to be a rare memory leak, I'm inclined to leave this
> till after 2.6.20.

Agreed.

> 
> --b.
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index dabd527..91284a0 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -199,18 +199,16 @@ defer_free(struct nfsd4_compoundargs *argp,
>  
>  static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
>  {
> -	void *new = NULL;
>  	if (p == argp->tmp) {
> -		new = kmalloc(nbytes, GFP_KERNEL);
> -		if (!new) return NULL;
> -		p = new;
> +		p = kmalloc(nbytes, GFP_KERNEL);
> +		if (!p) return NULL;
>  		memcpy(p, argp->tmp, nbytes);
>  	} else {
>  		BUG_ON(p != argp->tmpp);
>  		argp->tmpp = NULL;
>  	}
>  	if (defer_free(argp, kfree, p)) {
> -		kfree(new);
> +		kfree(p);
>  		return NULL;
>  	} else
>  		return (char *)p;
> 

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2006-12-31 20:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-27 13:18 [PATCH] kfree in fs/nfsd/nfs4xdr.c:savemem() Benny Halevy
2006-12-29 22:58 ` J. Bruce Fields
2006-12-31 20:22   ` Halevy, Benny

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.