[PATCH] net/sunrpc/svcauth_unix.c: unix_domain_find: return NULL if kmalloc fails

[PATCH] net/sunrpc/svcauth_unix.c: unix_domain_find: return NULL if kmalloc fails

[Petri T. Koistinen]

Hi!

I browsed http://linuxbugs.coverity.com/ site and found this:

NULL_RETURNS error: Dereference of possibly NULL ptr "new" returned by "kmalloc".
/home/test/nightly-qa/test-packages/linux-2.6.4/net/sunrpc/svcauth_unix.c:53:unix_domain_find:
(also see line 54)
53            new = kmalloc(sizeof(*new), GFP_KERNEL);
NULL_RETURNS information here
(also see line 53)
54            cache_init(&new->h.h);

Is this correct fix? What happens when unix_domain_find return NULL?

Best regards,
Petri Koistinen

--- linux-2.5/net/sunrpc/svcauth_unix.c.orig	2004-04-26 18:58:04.000000000 +0300
+++ linux-2.5/net/sunrpc/svcauth_unix.c	2004-04-26 18:58:58.000000000 +0300
@@ -36,36 +36,38 @@ struct unix_domain {
 struct auth_domain *unix_domain_find(char *name)
 {
 	struct auth_domain *rv, ud;
 	struct unix_domain *new;

 	ud.name = name;

 	rv = auth_domain_lookup(&ud, 0);

  foundit:
 	if (rv && rv->flavour != RPC_AUTH_UNIX) {
 		auth_domain_put(rv);
 		return NULL;
 	}
 	if (rv)
 		return rv;

 	new = kmalloc(sizeof(*new), GFP_KERNEL);
+	if (new == NULL)
+		return NULL;
 	cache_init(&new->h.h);
 	atomic_inc(&new->h.h.refcnt);
 	new->h.name = strdup(name);
 	new->h.flavour = RPC_AUTH_UNIX;
 	new->addr_changes = 0;
 	new->h.h.expiry_time = NEVER;
 	new->h.h.flags = 0;

 	rv = auth_domain_lookup(&new->h, 2);
 	if (rv == &new->h) {
 		if (atomic_dec_and_test(&new->h.h.refcnt)) BUG();
 	} else {
 		auth_domain_put(&new->h);
 		goto foundit;
 	}

 	return rv;
 }

Re: [PATCH] net/sunrpc/svcauth_unix.c: unix_domain_find: return NULL if kmalloc fails

[J. Bruce Fields]

On Mon, Apr 26, 2004 at 07:17:19PM +0300, Petri T. Koistinen wrote:
> Is this correct fix? What happens when unix_domain_find return NULL?

I just noticed that the other day, and have been testing with the
identical fix; seems to work fine.  All the callers check for NULL
returns and appear to do the right thing.  --Bruce Fields

> --- linux-2.5/net/sunrpc/svcauth_unix.c.orig	2004-04-26 18:58:04.000000000 +0300
> +++ linux-2.5/net/sunrpc/svcauth_unix.c	2004-04-26 18:58:58.000000000 +0300
> @@ -36,36 +36,38 @@ struct unix_domain {
>  struct auth_domain *unix_domain_find(char *name)
>  {
>  	struct auth_domain *rv, ud;
>  	struct unix_domain *new;
> 
>  	ud.name = name;
> 
>  	rv = auth_domain_lookup(&ud, 0);
> 
>   foundit:
>  	if (rv && rv->flavour != RPC_AUTH_UNIX) {
>  		auth_domain_put(rv);
>  		return NULL;
>  	}
>  	if (rv)
>  		return rv;
> 
>  	new = kmalloc(sizeof(*new), GFP_KERNEL);
> +	if (new == NULL)
> +		return NULL;
>  	cache_init(&new->h.h);
>  	atomic_inc(&new->h.h.refcnt);
>  	new->h.name = strdup(name);
>  	new->h.flavour = RPC_AUTH_UNIX;
>  	new->addr_changes = 0;
>  	new->h.h.expiry_time = NEVER;
>  	new->h.h.flags = 0;
> 
>  	rv = auth_domain_lookup(&new->h, 2);
>  	if (rv == &new->h) {
>  		if (atomic_dec_and_test(&new->h.h.refcnt)) BUG();
>  	} else {
>  		auth_domain_put(&new->h);
>  		goto foundit;
>  	}
> 
>  	return rv;
>  }
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] net/sunrpc/svcauth_unix.c: unix_domain_find: return NULL if kmalloc fails

[David S. Miller]

On Mon, 26 Apr 2004 19:17:19 +0300 (EEST)
"Petri T. Koistinen" <petri.koistinen@iki.fi> wrote:

> I browsed http://linuxbugs.coverity.com/ site and found this:
> 
> NULL_RETURNS error: Dereference of possibly NULL ptr "new" returned by "kmalloc".
 ...
> Is this correct fix? What happens when unix_domain_find return NULL?

Re: [PATCH] net/sunrpc/svcauth_unix.c: unix_domain_find: return NULL if kmalloc fails

[David S. Miller]

On Mon, 26 Apr 2004 19:17:19 +0300 (EEST)
"Petri T. Koistinen" <petri.koistinen@iki.fi> wrote:

> I browsed http://linuxbugs.coverity.com/ site and found this:
 ...
> Is this correct fix? What happens when unix_domain_find return NULL?

I've put this into my tree and will pass on to Linus.