[PATCH] vfs: use kstrdup()

[PATCH] vfs: use kstrdup()

[Li Zefan]

This piece of code can be replaced by a kstrdup().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 fs/namespace.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4f6f763..08c3b2d 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -127,14 +127,7 @@ struct vfsmount *alloc_vfsmnt(const char *name)
 		INIT_LIST_HEAD(&mnt->mnt_slave_list);
 		INIT_LIST_HEAD(&mnt->mnt_slave);
 		atomic_set(&mnt->__mnt_writers, 0);
-		if (name) {
-			int size = strlen(name) + 1;
-			char *newname = kmalloc(size, GFP_KERNEL);
-			if (newname) {
-				memcpy(newname, name, size);
-				mnt->mnt_devname = newname;
-			}
-		}
+		mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
 	}
 	return mnt;
 }
-- 
1.5.4.rc3

Re: [PATCH] vfs: use kstrdup()

[Pekka Enberg]

On Sat, Jul 19, 2008 at 1:16 PM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> This piece of code can be replaced by a kstrdup().
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Looks good to me.

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

> ---
>  fs/namespace.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 4f6f763..08c3b2d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -127,14 +127,7 @@ struct vfsmount *alloc_vfsmnt(const char *name)
>                INIT_LIST_HEAD(&mnt->mnt_slave_list);
>                INIT_LIST_HEAD(&mnt->mnt_slave);
>                atomic_set(&mnt->__mnt_writers, 0);
> -               if (name) {
> -                       int size = strlen(name) + 1;
> -                       char *newname = kmalloc(size, GFP_KERNEL);
> -                       if (newname) {
> -                               memcpy(newname, name, size);
> -                               mnt->mnt_devname = newname;
> -                       }
> -               }
> +               mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
>        }
>        return mnt;
>  }
> --
> 1.5.4.rc3
>
>
> --
> 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] vfs: use kstrdup()

[Cyrill Gorcunov]

[Li Zefan - Sat, Jul 19, 2008 at 06:16:20PM +0800]
| This piece of code can be replaced by a kstrdup().
| 
| Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
| ---
|  fs/namespace.c |    9 +--------
|  1 files changed, 1 insertions(+), 8 deletions(-)
| 
| diff --git a/fs/namespace.c b/fs/namespace.c
| index 4f6f763..08c3b2d 100644
| --- a/fs/namespace.c
| +++ b/fs/namespace.c
| @@ -127,14 +127,7 @@ struct vfsmount *alloc_vfsmnt(const char *name)
|  		INIT_LIST_HEAD(&mnt->mnt_slave_list);
|  		INIT_LIST_HEAD(&mnt->mnt_slave);
|  		atomic_set(&mnt->__mnt_writers, 0);
| -		if (name) {
| -			int size = strlen(name) + 1;
| -			char *newname = kmalloc(size, GFP_KERNEL);
| -			if (newname) {
| -				memcpy(newname, name, size);
| -				mnt->mnt_devname = newname;
| -			}
| -		}
| +		mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
|  	}
|  	return mnt;
|  }
| -- 
| 1.5.4.rc3
| 
| 

but kstrdup may return NULL - is it safe there?
Sorry if that "not smart" question.

		- Cyrill -

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Cyrill Gorcunov - Sat, Jul 19, 2008 at 05:13:17PM +0400]
[...]
| | -			}
| | -		}
| | +		mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| |  	}
| |  	return mnt;
| |  }
| | -- 
| | 1.5.4.rc3
| | 
| | 
| 
| but kstrdup may return NULL - is it safe there?
| Sorry if that "not smart" question.
| 
| 		- Cyrill -

ah, I see it is safe - sorry for noise

		- Cyrill -

Re: [PATCH] vfs: use kstrdup()

[Al Viro]

On Sat, Jul 19, 2008 at 05:19:09PM +0400, Cyrill Gorcunov wrote:
> [Cyrill Gorcunov - Sat, Jul 19, 2008 at 05:13:17PM +0400]
> [...]
> | | -			}
> | | -		}
> | | +		mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
> | |  	}
> | |  	return mnt;
> | |  }
> | | -- 
> | | 1.5.4.rc3
> | | 
> | | 
> | 
> | but kstrdup may return NULL - is it safe there?
> | Sorry if that "not smart" question.
> | 
> | 		- Cyrill -
> 
> ah, I see it is safe - sorry for noise

FWIW, it _is_ a good question.

	* is all code treating ->mnt_devname as optional?  AFAICS, there's
at least one place in NFS that doesn't.  We could treat failing allocation
the same way we treat failing allocation of vfsmount itself - callers can
cope with that already.
	* AFAICS, it should be const char *.
	* ... or perhaps we shouldn't copy it at all.  How about something
like
	struct {
		int count;
		char name[];
	}
with cloning sharing the reference and bumping the count, protecting it with
e.g. vfsmount_lock?  For setups where we have a lot of bindings/namespaces
it might be noticable.

Any takers?

Re: [PATCH] vfs: use kstrdup()

[Li Zefan]

Al Viro wrote:
> On Sat, Jul 19, 2008 at 05:19:09PM +0400, Cyrill Gorcunov wrote:
>> [Cyrill Gorcunov - Sat, Jul 19, 2008 at 05:13:17PM +0400]
>> [...]
>> | | -			}
>> | | -		}
>> | | +		mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
>> | |  	}
>> | |  	return mnt;
>> | |  }
>> | | -- 
>> | | 1.5.4.rc3
>> | | 
>> | | 
>> | 
>> | but kstrdup may return NULL - is it safe there?
>> | Sorry if that "not smart" question.
>> | 
>> | 		- Cyrill -
>>
>> ah, I see it is safe - sorry for noise
> 
> FWIW, it _is_ a good question.
> 
> 	* is all code treating ->mnt_devname as optional?  AFAICS, there's
> at least one place in NFS that doesn't.  We could treat failing allocation
> the same way we treat failing allocation of vfsmount itself - callers can
> cope with that already.

I just did a cleanup, and the original code didn't check for NULL.

I just looked into the git history, and I found out since fs/namespace.c was
created in v2.4.10.4, the code has never changed to check for failing
allocation of ->mnt_devname.

> 	* AFAICS, it should be const char *.

Agreed.

> 	* ... or perhaps we shouldn't copy it at all.  How about something
> like
> 	struct {
> 		int count;
> 		char name[];
> 	}
> with cloning sharing the reference and bumping the count, protecting it with
> e.g. vfsmount_lock?  For setups where we have a lot of bindings/namespaces
> it might be noticable.
> 

I'm not sure whether this is a good idea, as I have limited knowledge about the
vfs internal.

Re: [PATCH] vfs: use kstrdup()

[Al Viro]

On Mon, Jul 21, 2008 at 02:29:47PM +0800, Li Zefan wrote:
> > FWIW, it _is_ a good question.
> > 
> > 	* is all code treating ->mnt_devname as optional?  AFAICS, there's
> > at least one place in NFS that doesn't.  We could treat failing allocation
> > the same way we treat failing allocation of vfsmount itself - callers can
> > cope with that already.
> 
> I just did a cleanup, and the original code didn't check for NULL.

I know.

> I just looked into the git history, and I found out since fs/namespace.c was
> created in v2.4.10.4, the code has never changed to check for failing
> allocation of ->mnt_devname.

It used to have no users beyond fs/namespace.c itself and for _those_ the
thing had been optional, so leaving NULL had been OK.  Unfortunately, it
still had been a bad idea - new users had appeared and those predictably
didn't notice that fun detail.

The right thing here is to consider failing allocation of ->mnt_devname
as failure of the entire alloc.

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
| On Mon, Jul 21, 2008 at 02:29:47PM +0800, Li Zefan wrote:
| > > FWIW, it _is_ a good question.
| > > 
| > > 	* is all code treating ->mnt_devname as optional?  AFAICS, there's
| > > at least one place in NFS that doesn't.  We could treat failing allocation
| > > the same way we treat failing allocation of vfsmount itself - callers can
| > > cope with that already.
| > 
| > I just did a cleanup, and the original code didn't check for NULL.
| 
| I know.
| 
| > I just looked into the git history, and I found out since fs/namespace.c was
| > created in v2.4.10.4, the code has never changed to check for failing
| > allocation of ->mnt_devname.
| 
| It used to have no users beyond fs/namespace.c itself and for _those_ the
| thing had been optional, so leaving NULL had been OK.  Unfortunately, it
| still had been a bad idea - new users had appeared and those predictably
| didn't notice that fun detail.
| 
| The right thing here is to consider failing allocation of ->mnt_devname
| as failure of the entire alloc.
|

Hi Al,

thanks a lot for comments! I think it is more then enough for now (i'm
about failing allocation in whole). If that happens and we are not able
to duplicate string - it's quite probable we will be in serious troubles
soon anyway ('cause of further kmalloc calls). So it's better to get
mount allocation fails then NULL deref.
 
		- Cyrill -

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
| 
[...]
| The right thing here is to consider failing allocation of ->mnt_devname
| as failure of the entire alloc.
| 

Al, what about the patch below? I'm not sure if Li's version already
in someone tree so it's from-the-scratch. If this ok, i think Li could
update his version and resend.

		- Cyrill -
---

Index: linux-2.6.git/fs/namespace.c
===================================================================
--- linux-2.6.git.orig/fs/namespace.c	2008-07-21 11:34:37.000000000 +0400
+++ linux-2.6.git/fs/namespace.c	2008-07-21 12:00:01.000000000 +0400
@@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
 		int err;
 
 		err = mnt_alloc_id(mnt);
-		if (err) {
-			kmem_cache_free(mnt_cache, mnt);
-			return NULL;
+		if (err)
+			goto err;
+
+		if (name) {
+			mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
+			if (!mnt->mnt_devname)
+				goto err;
 		}
 
 		atomic_set(&mnt->mnt_count, 1);
@@ -127,16 +131,12 @@ struct vfsmount *alloc_vfsmnt(const char
 		INIT_LIST_HEAD(&mnt->mnt_slave_list);
 		INIT_LIST_HEAD(&mnt->mnt_slave);
 		atomic_set(&mnt->__mnt_writers, 0);
-		if (name) {
-			int size = strlen(name) + 1;
-			char *newname = kmalloc(size, GFP_KERNEL);
-			if (newname) {
-				memcpy(newname, name, size);
-				mnt->mnt_devname = newname;
-			}
-		}
 	}
 	return mnt;
+
+err:
+	kmem_cache_free(mnt_cache, mnt);
+	return NULL;
 }
 
 /*

Re: [PATCH] vfs: use kstrdup()

[Li Zefan]

Cyrill Gorcunov wrote:
> [Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
> | 
> [...]
> | The right thing here is to consider failing allocation of ->mnt_devname
> | as failure of the entire alloc.
> | 
> 
> Al, what about the patch below? I'm not sure if Li's version already
> in someone tree so it's from-the-scratch. If this ok, i think Li could
> update his version and resend.
> 

It's already in -mm tree, but Andrew can drop it and queue the new one.

> 		- Cyrill -
> ---
> 
> Index: linux-2.6.git/fs/namespace.c
> ===================================================================
> --- linux-2.6.git.orig/fs/namespace.c	2008-07-21 11:34:37.000000000 +0400
> +++ linux-2.6.git/fs/namespace.c	2008-07-21 12:00:01.000000000 +0400
> @@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
>  		int err;
>  
>  		err = mnt_alloc_id(mnt);
> -		if (err) {
> -			kmem_cache_free(mnt_cache, mnt);
> -			return NULL;
> +		if (err)
> +			goto err;
> +
> +		if (name) {
> +			mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
> +			if (!mnt->mnt_devname)

should call mnt_free_id() here.

> +				goto err;
>  		}
>  
>  		atomic_set(&mnt->mnt_count, 1);
> @@ -127,16 +131,12 @@ struct vfsmount *alloc_vfsmnt(const char
>  		INIT_LIST_HEAD(&mnt->mnt_slave_list);
>  		INIT_LIST_HEAD(&mnt->mnt_slave);
>  		atomic_set(&mnt->__mnt_writers, 0);
> -		if (name) {
> -			int size = strlen(name) + 1;
> -			char *newname = kmalloc(size, GFP_KERNEL);
> -			if (newname) {
> -				memcpy(newname, name, size);
> -				mnt->mnt_devname = newname;
> -			}
> -		}
>  	}
>  	return mnt;
> +
> +err:
> +	kmem_cache_free(mnt_cache, mnt);
> +	return NULL;
>  }
>  
>  /*
> 
> 

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Li Zefan - Mon, Jul 21, 2008 at 04:28:17PM +0800]
| Cyrill Gorcunov wrote:
| > [Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
| > | 
| > [...]
| > | The right thing here is to consider failing allocation of ->mnt_devname
| > | as failure of the entire alloc.
| > | 
| > 
| > Al, what about the patch below? I'm not sure if Li's version already
| > in someone tree so it's from-the-scratch. If this ok, i think Li could
| > update his version and resend.
| > 
| 
| It's already in -mm tree, but Andrew can drop it and queue the new one.
| 
| > 		- Cyrill -
| > ---
| > 
| > Index: linux-2.6.git/fs/namespace.c
| > ===================================================================
| > --- linux-2.6.git.orig/fs/namespace.c	2008-07-21 11:34:37.000000000 +0400
| > +++ linux-2.6.git/fs/namespace.c	2008-07-21 12:00:01.000000000 +0400
| > @@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
| >  		int err;
| >  
| >  		err = mnt_alloc_id(mnt);
| > -		if (err) {
| > -			kmem_cache_free(mnt_cache, mnt);
| > -			return NULL;
| > +		if (err)
| > +			goto err;
| > +
| > +		if (name) {
| > +			mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| > +			if (!mnt->mnt_devname)
| 
| should call mnt_free_id() here.
| 
| > +				goto err;
| >  		}
| >  
| >  		atomic_set(&mnt->mnt_count, 1);
| > @@ -127,16 +131,12 @@ struct vfsmount *alloc_vfsmnt(const char
| >  		INIT_LIST_HEAD(&mnt->mnt_slave_list);
| >  		INIT_LIST_HEAD(&mnt->mnt_slave);
| >  		atomic_set(&mnt->__mnt_writers, 0);
| > -		if (name) {
| > -			int size = strlen(name) + 1;
| > -			char *newname = kmalloc(size, GFP_KERNEL);
| > -			if (newname) {
| > -				memcpy(newname, name, size);
| > -				mnt->mnt_devname = newname;
| > -			}
| > -		}
| >  	}
| >  	return mnt;
| > +
| > +err:
| > +	kmem_cache_free(mnt_cache, mnt);
| > +	return NULL;
| >  }
| >  
| >  /*
| > 
| > 
|

well, as only Al find new version more or less correct we could
update yours.
 
		- Cyrill -

Re: [PATCH] vfs: use kstrdup()

[Li Zefan]

> well, as only Al find new version more or less correct we could
> update yours.
>  

Sorry, I don't quite catch what you mean for my poor English. :(

Do you expect me to update and resend my version? or do you mean don't
drop the patch in -mm but you will resend a patch based against it?

Re: [PATCH] vfs: use kstrdup()

[Pekka Enberg]

Hi,

On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>> well, as only Al find new version more or less correct we could
>> update yours.
>
> Sorry, I don't quite catch what you mean for my poor English. :(
>
> Do you expect me to update and resend my version? or do you mean don't
> drop the patch in -mm but you will resend a patch based against it?

Andrew already merged your patch so I think Cyrill should just send a
new patch against -mm.

                          Pekka

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Pekka Enberg - Mon, Jul 21, 2008 at 12:06:55PM +0300]
| Hi,
| 
| On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
| >> well, as only Al find new version more or less correct we could
| >> update yours.
| >
| > Sorry, I don't quite catch what you mean for my poor English. :(
| >
| > Do you expect me to update and resend my version? or do you mean don't
| > drop the patch in -mm but you will resend a patch based against it?
| 
| Andrew already merged your patch so I think Cyrill should just send a
| new patch against -mm.
| 
|                           Pekka
|

Thanks guys, will send patch against -mm soon (a bit busy now)
 
		- Cyrill -

Re: [PATCH] vfs: use kstrdup()

[Li Zefan]

Cyrill Gorcunov wrote:
> [Pekka Enberg - Mon, Jul 21, 2008 at 12:06:55PM +0300]
> | Hi,
> | 
> | On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> | >> well, as only Al find new version more or less correct we could
> | >> update yours.
> | >
> | > Sorry, I don't quite catch what you mean for my poor English. :(
> | >
> | > Do you expect me to update and resend my version? or do you mean don't
> | > drop the patch in -mm but you will resend a patch based against it?
> | 
> | Andrew already merged your patch so I think Cyrill should just send a
> | new patch against -mm.
> | 
> |                           Pekka
> |
> 
> Thanks guys, will send patch against -mm soon (a bit busy now)
>  

Oh, I just sent out a mix-up patch because I'm going to get off work but I didn't
get your respond, you can just ignore it.

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Li Zefan - Mon, Jul 21, 2008 at 06:10:19PM +0800]
| Cyrill Gorcunov wrote:
| > [Pekka Enberg - Mon, Jul 21, 2008 at 12:06:55PM +0300]
| > | Hi,
| > | 
| > | On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
| > | >> well, as only Al find new version more or less correct we could
| > | >> update yours.
| > | >
| > | > Sorry, I don't quite catch what you mean for my poor English. :(
| > | >
| > | > Do you expect me to update and resend my version? or do you mean don't
| > | > drop the patch in -mm but you will resend a patch based against it?
| > | 
| > | Andrew already merged your patch so I think Cyrill should just send a
| > | new patch against -mm.
| > | 
| > |                           Pekka
| > |
| > 
| > Thanks guys, will send patch against -mm soon (a bit busy now)
| >  
| 
| Oh, I just sent out a mix-up patch because I'm going to get off work but I didn't
| get your respond, you can just ignore it.
|

it's ok - since you did my work for me :) Thanks.
 
		- Cyrill -

Re: [PATCH] vfs: use kstrdup()

[Al Viro]

On Mon, Jul 21, 2008 at 04:59:16PM +0800, Li Zefan wrote:
> > well, as only Al find new version more or less correct we could
> > update yours.
> >  
> 
> Sorry, I don't quite catch what you mean for my poor English. :(
> 
> Do you expect me to update and resend my version? or do you mean don't
> drop the patch in -mm but you will resend a patch based against it?

<shrug>

FWIW, the latest posted variant looks sane.  Whether you want it in one
piece or two... up to you, folks.  Whichever way you do it, throw the
char * -> const char * transition for mnt_devname into the mix and send
the prefered splitup; I'll apply it after I get some sleep...

Re: [PATCH] vfs: use kstrdup()

[Al Viro]

On Mon, Jul 21, 2008 at 12:04:27PM +0400, Cyrill Gorcunov wrote:
>  		err = mnt_alloc_id(mnt);
> -		if (err) {
> -			kmem_cache_free(mnt_cache, mnt);
> -			return NULL;
> +		if (err)
> +			goto err;

Ugh...  Labels are in a separate namespace, but really...

> +		if (name) {
> +			mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
> +			if (!mnt->mnt_devname)
> +				goto err;

> +err:
> +	kmem_cache_free(mnt_cache, mnt);
> +	return NULL;

Leak; note the mnt_alloc_id() above.  Either do that kstrdup() first and
kfree the result on mnt_alloc_id() failure or do mnt_free_id() on kstrdup()
one...

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Al Viro - Mon, Jul 21, 2008 at 09:28:57AM +0100]
| On Mon, Jul 21, 2008 at 12:04:27PM +0400, Cyrill Gorcunov wrote:
| >  		err = mnt_alloc_id(mnt);
| > -		if (err) {
| > -			kmem_cache_free(mnt_cache, mnt);
| > -			return NULL;
| > +		if (err)
| > +			goto err;
| 
| Ugh...  Labels are in a separate namespace, but really...
| 
| > +		if (name) {
| > +			mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| > +			if (!mnt->mnt_devname)
| > +				goto err;
| 
| > +err:
| > +	kmem_cache_free(mnt_cache, mnt);
| > +	return NULL;
| 
| Leak; note the mnt_alloc_id() above.  Either do that kstrdup() first and
| kfree the result on mnt_alloc_id() failure or do mnt_free_id() on kstrdup()
| one...
| 

oh my, that stupid from my side - will update shortly

		- Cyrill -

Re: [PATCH] vfs: use kstrdup()

[Cyrill Gorcunov]

[Al Viro - Mon, Jul 21, 2008 at 09:28:57AM +0100]
| On Mon, Jul 21, 2008 at 12:04:27PM +0400, Cyrill Gorcunov wrote:
| >  		err = mnt_alloc_id(mnt);
| > -		if (err) {
| > -			kmem_cache_free(mnt_cache, mnt);
| > -			return NULL;
| > +		if (err)
| > +			goto err;
| 
| Ugh...  Labels are in a separate namespace, but really...
| 
| > +		if (name) {
| > +			mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| > +			if (!mnt->mnt_devname)
| > +				goto err;
| 
| > +err:
| > +	kmem_cache_free(mnt_cache, mnt);
| > +	return NULL;
| 
| Leak; note the mnt_alloc_id() above.  Either do that kstrdup() first and
| kfree the result on mnt_alloc_id() failure or do mnt_free_id() on kstrdup()
| one...
| 

ok, here is an update version. labels names could be not that good
choosen (check please).

		- Cyrill -
---

Index: linux-2.6.git/fs/namespace.c
===================================================================
--- linux-2.6.git.orig/fs/namespace.c	2008-07-21 11:34:37.000000000 +0400
+++ linux-2.6.git/fs/namespace.c	2008-07-21 12:40:12.000000000 +0400
@@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
 		int err;
 
 		err = mnt_alloc_id(mnt);
-		if (err) {
-			kmem_cache_free(mnt_cache, mnt);
-			return NULL;
+		if (err)
+			goto out_free_cache;
+
+		if (name) {
+			mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
+			if (!mnt->mnt_devname)
+				goto out_free_id;
 		}
 
 		atomic_set(&mnt->mnt_count, 1);
@@ -127,16 +131,14 @@ struct vfsmount *alloc_vfsmnt(const char
 		INIT_LIST_HEAD(&mnt->mnt_slave_list);
 		INIT_LIST_HEAD(&mnt->mnt_slave);
 		atomic_set(&mnt->__mnt_writers, 0);
-		if (name) {
-			int size = strlen(name) + 1;
-			char *newname = kmalloc(size, GFP_KERNEL);
-			if (newname) {
-				memcpy(newname, name, size);
-				mnt->mnt_devname = newname;
-			}
-		}
 	}
 	return mnt;
+
+out_free_id:
+	mnt_free_id(mnt);
+out_free_cache:
+	kmem_cache_free(mnt_cache, mnt);
+	return NULL;
 }
 
 /*