Linux-audit Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21 21:38 UTC (permalink / raw)
  To: Al Viro
  Cc: Sabrina Dubroca, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121212833.GS29656@ZenIV.linux.org.uk>

On Wed, Jan 21, 2015 at 09:28:33PM +0000, Al Viro wrote:
> On Wed, Jan 21, 2015 at 01:03:20PM -0800, Guenter Roeck wrote:
> > 
> > failing case:
> > 
> > path_lookupat: calling path_init 'usr' flags=40
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=40[50] returned 0
> > walk_component: lookup_fast() returned 1
> > walk_component: lookup_slow() returned 0
> > walk_component: inode=  (null), negative=1
> > do_path_lookup(usr, 0x10)
> > path_lookupat: calling path_init 'usr' flags=50
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=50[50] returned 0
> > mkdir[c74012a0,/kkk] => 0						<==== SIC!
> 
> Cute. 'k' being 0x6b, aka POISON_FREE...  OK, the next question is what's
> been freed under us - I don't believe that it's dentry itself...
> Oh, fuck.  OK, I see what happens.  Look at kern_path_create(); it does
> LOOKUP_PARENT walk, leaving nd->last pointing to the last component of
> the *COPY* of the name it's just created, walked and freed.
> 
Cool.

> OK...  Fortunately, struct nameidata is completely opaque outside of fs/namei.c,
> so we only need to care about a couple of codepaths.
> 
> Folks, could you check if the following on top of linux-next fixes the problem?
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 323957f..cda89c3 100644

Yes, this patch fixes the problem for me.

Guenter

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Sabrina Dubroca @ 2015-01-21 21:30 UTC (permalink / raw)
  To: Al Viro
  Cc: Guenter Roeck, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <54C01418.5060204@roeck-us.net>

2015-01-21, 13:03:20 -0800, Guenter Roeck wrote:
> On 01/21/2015 12:06 PM, Al Viro wrote:
> >On Wed, Jan 21, 2015 at 11:06:27AM -0800, Guenter Roeck wrote:
> >>On 01/21/2015 10:29 AM, Al Viro wrote:
> >>>On Wed, Jan 21, 2015 at 05:32:13AM -0800, Guenter Roeck wrote:
> >>>>Another data point (though I have no idea if it is useful or what it means):
> >>>>
> >>>>In the working case, path_init sets nd->flags to 0x50 or 0x51.
> >>>>In the non-working case (ie for all files with a '/' in the name),
> >>>>it sets nd->flags to 0x10 or 0x11, even though it is always called
> >>>>with the LOOKUP_RCU bit set in flags.
> >>>
> >>>Umm...  Are those path_init() succeeding or failing?  Note that path_init()
> >>>includes "walk everything except for the last component", so your non-working
> >>>case is "have it walk anything at all".  What's failing there?  path_init()
> >>>or handling the remaining component?
> >>>
> >>path_init() returns -2. Guess that explains the unexpected flags ;-).
> >>The failuere is from
> >>	link_path_walk()
> >>		walk_component()
> >
> >Which is to say, lookup gave it a negative dentry.  OK, let's just make
> >vfs_mkdir() and walk_component() print what they are doing; on top of
> >linux-next
> >
> >diff --git a/fs/namei.c b/fs/namei.c
> >index 323957f..8a4e22f 100644
> >--- a/fs/namei.c
> >+++ b/fs/namei.c
> >@@ -1586,8 +1586,11 @@ static inline int walk_component(struct nameidata *nd, struct path *path,
> >  		inode = path->dentry->d_inode;
> >  	}
> >  	err = -ENOENT;
> >-	if (!inode || d_is_negative(path->dentry))
> >+	if (!inode || d_is_negative(path->dentry)) {
> >+		printk(KERN_ERR "walk_component[%p,%pd4] -> negative\n",
> >+			path->dentry, path->dentry);
> >  		goto out_path_put;
> >+	}
> >
> >  	if (should_follow_link(path->dentry, follow)) {
> >  		if (nd->flags & LOOKUP_RCU) {
> >@@ -3521,6 +3524,7 @@ int vfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
> >  	error = dir->i_op->mkdir(dir, dentry, mode);
> >  	if (!error)
> >  		fsnotify_mkdir(dir, dentry);
> >+	printk(KERN_ERR "mkdir[%p,%pd4] => %d\n", dentry, dentry, error);
> >  	return error;
> >  }
> >  EXPORT_SYMBOL(vfs_mkdir);
> >
> 
> ok case (putname commented out):
> 
> user_path_at_empty lookup usr flags 0x0
> path_lookupat: calling path_init 'usr' flags=40
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=40[50] returned 0
> walk_component: lookup_fast() returned 1
> walk_component: lookup_slow() returned 0
> walk_component: inode=  (null), negative=1
> do_path_lookup(usr, 0x10)
> path_lookupat: calling path_init 'usr' flags=50
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=50[50] returned 0
> mkdir[c74012a0,/usr] => 0
> user_path_at_empty lookup usr flags 0x1
> path_lookupat: calling path_init 'usr' flags=41
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=41[51] returned 0
> walk_component: inode=c74004a0, negative=0
> user_path_at_empty lookup usr flags 0x1
> path_lookupat: calling path_init 'usr' flags=41
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=41[51] returned 0
> 
> failing case:
> 
> path_lookupat: calling path_init 'usr' flags=40
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=40[50] returned 0
> walk_component: lookup_fast() returned 1
> walk_component: lookup_slow() returned 0
> walk_component: inode=  (null), negative=1
> do_path_lookup(usr, 0x10)
> path_lookupat: calling path_init 'usr' flags=50
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=50[50] returned 0
> mkdir[c74012a0,/kkk] => 0						<==== SIC!
> user_path_at_empty lookup usr flags 0x1
> path_lookupat: calling path_init 'usr' flags=41
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=41[51] returned 0
> walk_component: lookup_fast() returned 1
> walk_component: lookup_slow() returned 0
> walk_component: inode=  (null), negative=1
> user_path_at_empty lookup usr flags 0x1
> path_lookupat: calling path_init 'usr' flags=41
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=41[51] returned 0
> walk_component: lookup_fast() returned 1
> walk_component: lookup_slow() returned 0
> walk_component: inode=  (null), negative=1

Yep, I get some "kkk" too.

With that patch:

## panic

[    0.544839] walk_component[ffff88001d6edbd8,/dev] -> negative
[    0.545507] mkdir[ffff88001d6ed1b8,/kkk] => 0
[    0.545886] sys_mkdir dev:40755 returned 0
[    0.546275] walk_component[ffff88001d6ec288,/dev] -> negative
[    0.546835] walk_component[ffff88001d6eca20,/dev] -> negative
[    0.547403] walk_component[ffff88001d6ed950,/dev] -> negative
[    0.547954] walk_component[ffff88001d6ed440,/dev] -> negative
[    0.549260] walk_component[ffff88001d6ec510,/dev] -> negative
[    0.551161] walk_component[ffff88001d6ec798,/dev] -> negative
[    0.551719] walk_component[ffff88001d6ed6c8,/dev] -> negative
[    0.552281] walk_component[ffff88001d6eef30,/root] -> negative
[    0.552866] mkdir[ffff88001d6ee000,/kkkk] => 0
[    0.553254] sys_mkdir root:40700 returned 0
[    0.553682] walk_component[ffff88001d6eeca8,/root] -> negative
[    0.554225] walk_component[ffff88001d6efbd8,/root] -> negative
[    0.554826] walk_component[ffff88001d6ef1b8,/root] -> negative
[    0.555390] walk_component[ffff88001d6ee288,/dev] -> negative
[    0.561523] walk_component[ffff88001d6eea20,/cpu] -> negative
[    0.562203] mkdir[ffff88001d6ef950,/kkk] => 0
[    0.562627] walk_component[ffff88001d6ef440,/cpu] -> negative
[    0.819060] walk_component[ffff88001d6df1b8,/input] -> negative
[    0.819672] mkdir[ffff88001d6def30,/kkkkk] => 0
[    0.820496] walk_component[ffff88001d6de510,/input] -> negative
[    0.840538] walk_component[ffff88001d6e6288,/dev] -> negative
[    0.841626] walk_component[ffff88001d6e6f30,/init] -> negative
[    0.992725] walk_component[ffff88001d6e6f30,/bsg] -> negative
[    0.993671] mkdir[ffff88001d6e6798,/kkk] => 0
[    0.994307] walk_component[ffff88001d6e6288,/bsg] -> negative
[    1.007701] walk_component[ffff88001d6e6ca8,/bsg] -> negative
[    1.008561] walk_component[ffff88001d6e7950,/bsg] -> negative
[    1.024292] walk_component[ffff88001d6e7950,/dev] -> negative
[    1.025017] walk_component[ffff88001d6e7440,/dev] -> negative
[    1.025781] walk_component[ffff88001d6e6f30,/dev] -> negative
[    1.026495] walk_component[ffff88001d6e6510,/dev] -> negative
[    1.027243] walk_component[ffff88001d6e6288,/dev] -> negative
[    1.027910] stat("/dev/root") -> -2
[    1.028358] walk_component[ffff88001d6df6c8,/dev] -> negative
[    1.029019] stat("dev") -> -2
[    1.029431] walk_component[ffff88001d6df1b8,/root] -> negative

## good

[    0.350197] walk_component[ffff88001e348c80,/dev] -> negative
[    0.350821] mkdir[ffff88001e348dc0,/dev] => 0
[    0.351467] sys_mkdir dev:40755 returned 0
[    0.351919] walk_component[ffff88001e349000,/dev/console] -> negative
[    0.353000] walk_component[ffff88001e349280,/root] -> negative
[    0.353959] mkdir[ffff88001e3493c0,/root] => 0
[    0.354354] sys_mkdir root:40700 returned 0
[    0.357927] walk_component[ffff88001e349500,/cpu] -> negative
[    0.358470] mkdir[ffff88001e349640,/cpu] => 0
[    0.472772] walk_component[ffff88001e34ba00,/input] -> negative
[    0.473392] mkdir[ffff88001e34bb40,/input] => 0
[    0.478682] walk_component[ffff88001e34cdc0,/init] -> negative
[    0.647559] walk_component[ffff88001e34cdc0,/bsg] -> negative
[    0.648671] mkdir[ffff88001e34ba00,/bsg] => 0
[    0.665831] stat("/dev/root") -> 0
[    0.666360] stat("dev") -> 0


Another thing, not sure if it's relevant: I can boot with the
do_lookup_path patch applied, and without commenting out putname(), if
I disable SLUB debugging with slub_debug=- (with CONFIG_SLUB_DEBUG=y
CONFIG_SLUB_DEBUG_ON=y)



-- 
Sabrina

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-21 21:28 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Sabrina Dubroca, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <54C01418.5060204@roeck-us.net>

On Wed, Jan 21, 2015 at 01:03:20PM -0800, Guenter Roeck wrote:
> ok case (putname commented out):
> 
> user_path_at_empty lookup usr flags 0x0
> path_lookupat: calling path_init 'usr' flags=40
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=40[50] returned 0
> walk_component: lookup_fast() returned 1
> walk_component: lookup_slow() returned 0
> walk_component: inode=  (null), negative=1
> do_path_lookup(usr, 0x10)
> path_lookupat: calling path_init 'usr' flags=50
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=50[50] returned 0
> mkdir[c74012a0,/usr] => 0
> user_path_at_empty lookup usr flags 0x1
> path_lookupat: calling path_init 'usr' flags=41
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=41[51] returned 0
> walk_component: inode=c74004a0, negative=0
> user_path_at_empty lookup usr flags 0x1
> path_lookupat: calling path_init 'usr' flags=41
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=41[51] returned 0
> 
> failing case:
> 
> path_lookupat: calling path_init 'usr' flags=40
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=40[50] returned 0
> walk_component: lookup_fast() returned 1
> walk_component: lookup_slow() returned 0
> walk_component: inode=  (null), negative=1
> do_path_lookup(usr, 0x10)
> path_lookupat: calling path_init 'usr' flags=50
> path_init: link_path_walk() returned 0
> path_lookupat: path_init 'usr' flags=50[50] returned 0
> mkdir[c74012a0,/kkk] => 0						<==== SIC!

Cute. 'k' being 0x6b, aka POISON_FREE...  OK, the next question is what's
been freed under us - I don't believe that it's dentry itself...
Oh, fuck.  OK, I see what happens.  Look at kern_path_create(); it does
LOOKUP_PARENT walk, leaving nd->last pointing to the last component of
the *COPY* of the name it's just created, walked and freed.

OK...  Fortunately, struct nameidata is completely opaque outside of fs/namei.c,
so we only need to care about a couple of codepaths.

Folks, could you check if the following on top of linux-next fixes the problem?

diff --git a/fs/namei.c b/fs/namei.c
index 323957f..cda89c3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2056,13 +2056,22 @@ static int do_path_lookup(int dfd, const char *name,
 /* does lookup, returns the object with parent locked */
 struct dentry *kern_path_locked(const char *name, struct path *path)
 {
+	struct filename *filename = getname_kernel(name);
 	struct nameidata nd;
 	struct dentry *d;
-	int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd);
-	if (err)
+	int err;
+
+	if (IS_ERR(filename))
+		return ERR_CAST(filename);
+
+	err = filename_lookup(AT_FDCWD, filename, LOOKUP_PARENT, &nd);
+	if (err) {
+		putname(filename);
 		return ERR_PTR(err);
+	}
 	if (nd.last_type != LAST_NORM) {
 		path_put(&nd.path);
+		putname(filename);
 		return ERR_PTR(-EINVAL);
 	}
 	mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
@@ -2070,9 +2079,11 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
 	if (IS_ERR(d)) {
 		mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
 		path_put(&nd.path);
+		putname(filename);
 		return d;
 	}
 	*path = nd.path;
+	putname(filename);
 	return d;
 }
 
@@ -3314,7 +3325,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 	return file;
 }
 
-struct dentry *kern_path_create(int dfd, const char *pathname,
+static struct dentry *filename_create(int dfd, struct filename *name,
 				struct path *path, unsigned int lookup_flags)
 {
 	struct dentry *dentry = ERR_PTR(-EEXIST);
@@ -3329,7 +3340,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname,
 	 */
 	lookup_flags &= LOOKUP_REVAL;
 
-	error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
+	error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
 	if (error)
 		return ERR_PTR(error);
 
@@ -3383,6 +3394,19 @@ out:
 	path_put(&nd.path);
 	return dentry;
 }
+
+struct dentry *kern_path_create(int dfd, const char *pathname,
+				struct path *path, unsigned int lookup_flags)
+{
+	struct filename *filename = getname_kernel(pathname);
+	struct dentry *res = ERR_CAST(filename);
+
+	if (!IS_ERR(filename)) {
+		res = filename_create(dfd, filename, path, lookup_flags);
+		putname(filename);
+	}
+	return res;
+}
 EXPORT_SYMBOL(kern_path_create);
 
 void done_path_create(struct path *path, struct dentry *dentry)
@@ -3401,7 +3425,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname,
 	struct dentry *res;
 	if (IS_ERR(tmp))
 		return ERR_CAST(tmp);
-	res = kern_path_create(dfd, tmp->name, path, lookup_flags);
+	res = filename_create(dfd, tmp, path, lookup_flags);
 	putname(tmp);
 	return res;
 }

^ permalink raw reply related

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21 21:03 UTC (permalink / raw)
  To: Al Viro
  Cc: Sabrina Dubroca, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121200626.GR29656@ZenIV.linux.org.uk>

On 01/21/2015 12:06 PM, Al Viro wrote:
> On Wed, Jan 21, 2015 at 11:06:27AM -0800, Guenter Roeck wrote:
>> On 01/21/2015 10:29 AM, Al Viro wrote:
>>> On Wed, Jan 21, 2015 at 05:32:13AM -0800, Guenter Roeck wrote:
>>>> Another data point (though I have no idea if it is useful or what it means):
>>>>
>>>> In the working case, path_init sets nd->flags to 0x50 or 0x51.
>>>> In the non-working case (ie for all files with a '/' in the name),
>>>> it sets nd->flags to 0x10 or 0x11, even though it is always called
>>>> with the LOOKUP_RCU bit set in flags.
>>>
>>> Umm...  Are those path_init() succeeding or failing?  Note that path_init()
>>> includes "walk everything except for the last component", so your non-working
>>> case is "have it walk anything at all".  What's failing there?  path_init()
>>> or handling the remaining component?
>>>
>> path_init() returns -2. Guess that explains the unexpected flags ;-).
>> The failuere is from
>> 	link_path_walk()
>> 		walk_component()
>
> Which is to say, lookup gave it a negative dentry.  OK, let's just make
> vfs_mkdir() and walk_component() print what they are doing; on top of
> linux-next
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 323957f..8a4e22f 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1586,8 +1586,11 @@ static inline int walk_component(struct nameidata *nd, struct path *path,
>   		inode = path->dentry->d_inode;
>   	}
>   	err = -ENOENT;
> -	if (!inode || d_is_negative(path->dentry))
> +	if (!inode || d_is_negative(path->dentry)) {
> +		printk(KERN_ERR "walk_component[%p,%pd4] -> negative\n",
> +			path->dentry, path->dentry);
>   		goto out_path_put;
> +	}
>
>   	if (should_follow_link(path->dentry, follow)) {
>   		if (nd->flags & LOOKUP_RCU) {
> @@ -3521,6 +3524,7 @@ int vfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>   	error = dir->i_op->mkdir(dir, dentry, mode);
>   	if (!error)
>   		fsnotify_mkdir(dir, dentry);
> +	printk(KERN_ERR "mkdir[%p,%pd4] => %d\n", dentry, dentry, error);
>   	return error;
>   }
>   EXPORT_SYMBOL(vfs_mkdir);
>

ok case (putname commented out):

user_path_at_empty lookup usr flags 0x0
path_lookupat: calling path_init 'usr' flags=40
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=40[50] returned 0
walk_component: lookup_fast() returned 1
walk_component: lookup_slow() returned 0
walk_component: inode=  (null), negative=1
do_path_lookup(usr, 0x10)
path_lookupat: calling path_init 'usr' flags=50
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=50[50] returned 0
mkdir[c74012a0,/usr] => 0
user_path_at_empty lookup usr flags 0x1
path_lookupat: calling path_init 'usr' flags=41
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=41[51] returned 0
walk_component: inode=c74004a0, negative=0
user_path_at_empty lookup usr flags 0x1
path_lookupat: calling path_init 'usr' flags=41
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=41[51] returned 0

failing case:

path_lookupat: calling path_init 'usr' flags=40
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=40[50] returned 0
walk_component: lookup_fast() returned 1
walk_component: lookup_slow() returned 0
walk_component: inode=  (null), negative=1
do_path_lookup(usr, 0x10)
path_lookupat: calling path_init 'usr' flags=50
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=50[50] returned 0
mkdir[c74012a0,/kkk] => 0						<==== SIC!
user_path_at_empty lookup usr flags 0x1
path_lookupat: calling path_init 'usr' flags=41
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=41[51] returned 0
walk_component: lookup_fast() returned 1
walk_component: lookup_slow() returned 0
walk_component: inode=  (null), negative=1
user_path_at_empty lookup usr flags 0x1
path_lookupat: calling path_init 'usr' flags=41
path_init: link_path_walk() returned 0
path_lookupat: path_init 'usr' flags=41[51] returned 0
walk_component: lookup_fast() returned 1
walk_component: lookup_slow() returned 0
walk_component: inode=  (null), negative=1

Guenter




^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-21 20:06 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Sabrina Dubroca, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <54BFF8B3.6060802@roeck-us.net>

On Wed, Jan 21, 2015 at 11:06:27AM -0800, Guenter Roeck wrote:
> On 01/21/2015 10:29 AM, Al Viro wrote:
> >On Wed, Jan 21, 2015 at 05:32:13AM -0800, Guenter Roeck wrote:
> >>Another data point (though I have no idea if it is useful or what it means):
> >>
> >>In the working case, path_init sets nd->flags to 0x50 or 0x51.
> >>In the non-working case (ie for all files with a '/' in the name),
> >>it sets nd->flags to 0x10 or 0x11, even though it is always called
> >>with the LOOKUP_RCU bit set in flags.
> >
> >Umm...  Are those path_init() succeeding or failing?  Note that path_init()
> >includes "walk everything except for the last component", so your non-working
> >case is "have it walk anything at all".  What's failing there?  path_init()
> >or handling the remaining component?
> >
> path_init() returns -2. Guess that explains the unexpected flags ;-).
> The failuere is from
> 	link_path_walk()
> 		walk_component()

Which is to say, lookup gave it a negative dentry.  OK, let's just make
vfs_mkdir() and walk_component() print what they are doing; on top of
linux-next

diff --git a/fs/namei.c b/fs/namei.c
index 323957f..8a4e22f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1586,8 +1586,11 @@ static inline int walk_component(struct nameidata *nd, struct path *path,
 		inode = path->dentry->d_inode;
 	}
 	err = -ENOENT;
-	if (!inode || d_is_negative(path->dentry))
+	if (!inode || d_is_negative(path->dentry)) {
+		printk(KERN_ERR "walk_component[%p,%pd4] -> negative\n",
+			path->dentry, path->dentry);
 		goto out_path_put;
+	}
 
 	if (should_follow_link(path->dentry, follow)) {
 		if (nd->flags & LOOKUP_RCU) {
@@ -3521,6 +3524,7 @@ int vfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	error = dir->i_op->mkdir(dir, dentry, mode);
 	if (!error)
 		fsnotify_mkdir(dir, dentry);
+	printk(KERN_ERR "mkdir[%p,%pd4] => %d\n", dentry, dentry, error);
 	return error;
 }
 EXPORT_SYMBOL(vfs_mkdir);

^ permalink raw reply related

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21 19:06 UTC (permalink / raw)
  To: Al Viro
  Cc: Sabrina Dubroca, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121182905.GQ29656@ZenIV.linux.org.uk>

On 01/21/2015 10:29 AM, Al Viro wrote:
> On Wed, Jan 21, 2015 at 05:32:13AM -0800, Guenter Roeck wrote:
>> Another data point (though I have no idea if it is useful or what it means):
>>
>> In the working case, path_init sets nd->flags to 0x50 or 0x51.
>> In the non-working case (ie for all files with a '/' in the name),
>> it sets nd->flags to 0x10 or 0x11, even though it is always called
>> with the LOOKUP_RCU bit set in flags.
>
> Umm...  Are those path_init() succeeding or failing?  Note that path_init()
> includes "walk everything except for the last component", so your non-working
> case is "have it walk anything at all".  What's failing there?  path_init()
> or handling the remaining component?
>
path_init() returns -2. Guess that explains the unexpected flags ;-).
The failuere is from
	link_path_walk()
		walk_component()

Guenter


^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-21 18:29 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Sabrina Dubroca, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <54BFAA5D.4040108@roeck-us.net>

On Wed, Jan 21, 2015 at 05:32:13AM -0800, Guenter Roeck wrote:
> Another data point (though I have no idea if it is useful or what it means):
> 
> In the working case, path_init sets nd->flags to 0x50 or 0x51.
> In the non-working case (ie for all files with a '/' in the name),
> it sets nd->flags to 0x10 or 0x11, even though it is always called
> with the LOOKUP_RCU bit set in flags.

Umm...  Are those path_init() succeeding or failing?  Note that path_init()
includes "walk everything except for the last component", so your non-working
case is "have it walk anything at all".  What's failing there?  path_init()
or handling the remaining component?

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21 17:51 UTC (permalink / raw)
  To: Al Viro, Paul Moore
  Cc: Sabrina Dubroca, Thierry Reding, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121173810.GP29656@ZenIV.linux.org.uk>

On 01/21/2015 09:38 AM, Al Viro wrote:
> On Wed, Jan 21, 2015 at 11:16:23AM -0500, Paul Moore wrote:
>> On Wednesday, January 21, 2015 04:54:07 PM Sabrina Dubroca wrote:
>>> 2015-01-21, 16:39:12 +0100, Thierry Reding wrote:
>>>> That doesn't seem to help, at least in my case.
>>>
>>> Same here.
>>
>> Okay, thanks for trying.  Sorry that didn't resolve things.
>>
>>> Well, it's probably not an audit issue.  I tried audit=0 on the
>>> commandline, and I just rebuilt a kernel with CONFIG_AUDIT=n, and it's
>>> still panicing.  This should have fixed any audit-related issue,
>>> right?
>>
>> Most likely.  Back to the code I go ...
>
> FWIW, I really wonder if populate_rootfs() (run ultimately from
> kernel_init(), by way of kernel_init_freeable(), do_basic_setup() and
> do_initcalls()) ends up with some side effects as far as struct filename
> are concerned...
>
> Note that if we _ever_ hit reuse logics there, we are going to get bogus
> matches asoddingplenty - *all* those sys_mkdir(), etc. are going to be
> with filenames in the same reused buffer.  So if anything in there leaks
> from one call to another, we are going to have a mess on hands.
>
> Another place where that can be a problem is devtmpfs - there's a kernel
> thread doing actual mkdir, mknod, etc. in that abomination and if _that_
> ends up accumulating aushit entries, we'll end up with interesting problems.
>
> Folks, could you print the value of audit_dummy_context() in populate_rootfs()
> and in drivers/base/devtmpfs.c:devtmpfsd()?
>
populate_rootfs: audit_dummy_context() returns 1

Guenter


^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-21 17:38 UTC (permalink / raw)
  To: Paul Moore
  Cc: Sabrina Dubroca, Thierry Reding, Guenter Roeck, Stephen Rothwell,
	linux-next, linux-kernel, linux-fsdevel, linux-audit,
	Richard Guy Briggs
In-Reply-To: <6942214.D1ODllqz3j@sifl>

On Wed, Jan 21, 2015 at 11:16:23AM -0500, Paul Moore wrote:
> On Wednesday, January 21, 2015 04:54:07 PM Sabrina Dubroca wrote:
> > 2015-01-21, 16:39:12 +0100, Thierry Reding wrote:
> > > That doesn't seem to help, at least in my case.
> > 
> > Same here.
> 
> Okay, thanks for trying.  Sorry that didn't resolve things.
> 
> > Well, it's probably not an audit issue.  I tried audit=0 on the
> > commandline, and I just rebuilt a kernel with CONFIG_AUDIT=n, and it's
> > still panicing.  This should have fixed any audit-related issue,
> > right?
> 
> Most likely.  Back to the code I go ...

FWIW, I really wonder if populate_rootfs() (run ultimately from
kernel_init(), by way of kernel_init_freeable(), do_basic_setup() and
do_initcalls()) ends up with some side effects as far as struct filename
are concerned...

Note that if we _ever_ hit reuse logics there, we are going to get bogus
matches asoddingplenty - *all* those sys_mkdir(), etc. are going to be
with filenames in the same reused buffer.  So if anything in there leaks
from one call to another, we are going to have a mess on hands.

Another place where that can be a problem is devtmpfs - there's a kernel
thread doing actual mkdir, mknod, etc. in that abomination and if _that_
ends up accumulating aushit entries, we'll end up with interesting problems.

Folks, could you print the value of audit_dummy_context() in populate_rootfs()
and in drivers/base/devtmpfs.c:devtmpfsd()?

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21 16:21 UTC (permalink / raw)
  To: Sabrina Dubroca, Paul Moore
  Cc: Thierry Reding, Al Viro, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121155407.GA18701@kria>

On 01/21/2015 07:54 AM, Sabrina Dubroca wrote:
> 2015-01-21, 16:39:12 +0100, Thierry Reding wrote:
>> On Wed, Jan 21, 2015 at 10:24:11AM -0500, Paul Moore wrote:
>>> On Wednesday, January 21, 2015 03:42:16 PM Thierry Reding wrote:
>>>> On Wed, Jan 21, 2015 at 12:05:39PM +0100, Sabrina Dubroca wrote:
>>>>> 2015-01-21, 04:36:38 +0000, Al Viro wrote:
>>>>>> On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
>>>>>>> With this patch:
>>>>>>>
>>>>>>> sys_mkdir .:40775 returned -17
>>>>>>> sys_mkdir usr:40775 returned 0
>>>>>>> sys_mkdir usr/lib:40775 returned 0
>>>>>>> sys_mkdir usr/share:40755 returned 0
>>>>>>> sys_mkdir usr/share/udhcpc:40755 returned 0
>>>>>>> sys_mkdir usr/bin:40775 returned 0
>>>>>>> sys_mkdir usr/sbin:40775 returned 0
>>>>>>> sys_mkdir mnt:40775 returned 0
>>>>>>> sys_mkdir proc:40775 returned 0
>>>>>>> sys_mkdir root:40775 returned 0
>>>>>>> sys_mkdir lib:40775 returned 0
>>>>>>> sys_mkdir lib/modules:40775 returned 0
>>>>>>> ...
>>>>>>>
>>>>>>> and the problem is fixed.
>>>>>
>>>>> This patch also works for me.
>>>>>
>>>>>> ... except that it simply confirms that something's fishy with
>>>>>> getname_kernel() of ->name of struct filename returned by getname().
>>>>>> IOW, I still do not understand the mechanism of breakage there.
>>>>>
>>>>> I'm not so sure about that.  I tried to copy name to a new string in
>>>>> do_path_lookup and that didn't help.
>>>>>
>>>>> Now, I've removed the
>>>>>
>>>>>          putname(filename);
>>>>>
>>>>> line from do_path_lookup and I don't get the panic.
>>>>
>>>> That would indicate that somehow the refcount got unbalanced. Looking
>>>> more closely it seems like the various audit_*() function do take a
>>>> reference, but maybe that's not enough.
>>>
>>> I'm thinking the same thing and I think the problem may be that
>>> __audit_reusename() is not bumping the filename->refcnt.  Can someone who is
>>> seeing this problem bump the refcnt in __audit_reusename()?
>>>
>>>    struct filename *
>>>    __audit_reusename(const __user char *uptr)
>>>    {
>>>          struct audit_context *context = current->audit_context;
>>>          struct audit_names *n;
>>>
>>>          list_for_each_entry(n, &context->names_list, list) {
>>>                  if (!n->name)
>>>                          continue;
>>>                  if (n->name->uptr == uptr) {
>>> +                       n->name->refcnt++;
>>>                          return n->name;
>>>                  }
>>>          }
>>>          return NULL;
>>>    }
>>
>> That doesn't seem to help, at least in my case.
>
> Same here.
>
> Well, it's probably not an audit issue.  I tried audit=0 on the
> commandline, and I just rebuilt a kernel with CONFIG_AUDIT=n, and it's
> still panicing.  This should have fixed any audit-related issue,
> right?
>
I don't have audit enabled, so I don't think that is the problem either
(the refcount increase didn't help, and a WARN(1) added to the code
at the same location did not trigger).

Wonder if we have a use-after-free case and just have been lucky all along.

Guenter


^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Paul Moore @ 2015-01-21 16:16 UTC (permalink / raw)
  To: Sabrina Dubroca
  Cc: Thierry Reding, Al Viro, Guenter Roeck, Stephen Rothwell,
	linux-next, linux-kernel, linux-fsdevel, linux-audit,
	Richard Guy Briggs
In-Reply-To: <20150121155407.GA18701@kria>

On Wednesday, January 21, 2015 04:54:07 PM Sabrina Dubroca wrote:
> 2015-01-21, 16:39:12 +0100, Thierry Reding wrote:
> > That doesn't seem to help, at least in my case.
> 
> Same here.

Okay, thanks for trying.  Sorry that didn't resolve things.

> Well, it's probably not an audit issue.  I tried audit=0 on the
> commandline, and I just rebuilt a kernel with CONFIG_AUDIT=n, and it's
> still panicing.  This should have fixed any audit-related issue,
> right?

Most likely.  Back to the code I go ...

-- 
paul moore
security @ redhat

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Sabrina Dubroca @ 2015-01-21 15:54 UTC (permalink / raw)
  To: Paul Moore
  Cc: Thierry Reding, Al Viro, Guenter Roeck, Stephen Rothwell,
	linux-next, linux-kernel, linux-fsdevel, linux-audit,
	Richard Guy Briggs
In-Reply-To: <20150121153910.GD22884@ulmo.nvidia.com>

2015-01-21, 16:39:12 +0100, Thierry Reding wrote:
> On Wed, Jan 21, 2015 at 10:24:11AM -0500, Paul Moore wrote:
> > On Wednesday, January 21, 2015 03:42:16 PM Thierry Reding wrote:
> > > On Wed, Jan 21, 2015 at 12:05:39PM +0100, Sabrina Dubroca wrote:
> > > > 2015-01-21, 04:36:38 +0000, Al Viro wrote:
> > > > > On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
> > > > > > With this patch:
> > > > > > 
> > > > > > sys_mkdir .:40775 returned -17
> > > > > > sys_mkdir usr:40775 returned 0
> > > > > > sys_mkdir usr/lib:40775 returned 0
> > > > > > sys_mkdir usr/share:40755 returned 0
> > > > > > sys_mkdir usr/share/udhcpc:40755 returned 0
> > > > > > sys_mkdir usr/bin:40775 returned 0
> > > > > > sys_mkdir usr/sbin:40775 returned 0
> > > > > > sys_mkdir mnt:40775 returned 0
> > > > > > sys_mkdir proc:40775 returned 0
> > > > > > sys_mkdir root:40775 returned 0
> > > > > > sys_mkdir lib:40775 returned 0
> > > > > > sys_mkdir lib/modules:40775 returned 0
> > > > > > ...
> > > > > > 
> > > > > > and the problem is fixed.
> > > > 
> > > > This patch also works for me.
> > > > 
> > > > > ... except that it simply confirms that something's fishy with
> > > > > getname_kernel() of ->name of struct filename returned by getname(). 
> > > > > IOW, I still do not understand the mechanism of breakage there.
> > > > 
> > > > I'm not so sure about that.  I tried to copy name to a new string in
> > > > do_path_lookup and that didn't help.
> > > > 
> > > > Now, I've removed the
> > > > 
> > > >         putname(filename);
> > > > 
> > > > line from do_path_lookup and I don't get the panic.
> > > 
> > > That would indicate that somehow the refcount got unbalanced. Looking
> > > more closely it seems like the various audit_*() function do take a
> > > reference, but maybe that's not enough.
> > 
> > I'm thinking the same thing and I think the problem may be that 
> > __audit_reusename() is not bumping the filename->refcnt.  Can someone who is 
> > seeing this problem bump the refcnt in __audit_reusename()?
> > 
> >   struct filename *
> >   __audit_reusename(const __user char *uptr)
> >   {
> >         struct audit_context *context = current->audit_context;
> >         struct audit_names *n;
> > 
> >         list_for_each_entry(n, &context->names_list, list) {
> >                 if (!n->name)
> >                         continue;
> >                 if (n->name->uptr == uptr) {
> > +                       n->name->refcnt++;
> >                         return n->name;
> >                 }
> >         }
> >         return NULL;
> >   }
> 
> That doesn't seem to help, at least in my case.

Same here.

Well, it's probably not an audit issue.  I tried audit=0 on the
commandline, and I just rebuilt a kernel with CONFIG_AUDIT=n, and it's
still panicing.  This should have fixed any audit-related issue,
right?

-- 
Sabrina

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Thierry Reding @ 2015-01-21 15:39 UTC (permalink / raw)
  To: Paul Moore
  Cc: Al Viro, Sabrina Dubroca, Guenter Roeck, Stephen Rothwell,
	linux-next, linux-kernel, linux-fsdevel, linux-audit,
	Richard Guy Briggs
In-Reply-To: <9422577.FVVyjQBZjy@sifl>

[-- Attachment #1: Type: text/plain, Size: 2522 bytes --]

On Wed, Jan 21, 2015 at 10:24:11AM -0500, Paul Moore wrote:
> On Wednesday, January 21, 2015 03:42:16 PM Thierry Reding wrote:
> > On Wed, Jan 21, 2015 at 12:05:39PM +0100, Sabrina Dubroca wrote:
> > > 2015-01-21, 04:36:38 +0000, Al Viro wrote:
> > > > On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
> > > > > With this patch:
> > > > > 
> > > > > sys_mkdir .:40775 returned -17
> > > > > sys_mkdir usr:40775 returned 0
> > > > > sys_mkdir usr/lib:40775 returned 0
> > > > > sys_mkdir usr/share:40755 returned 0
> > > > > sys_mkdir usr/share/udhcpc:40755 returned 0
> > > > > sys_mkdir usr/bin:40775 returned 0
> > > > > sys_mkdir usr/sbin:40775 returned 0
> > > > > sys_mkdir mnt:40775 returned 0
> > > > > sys_mkdir proc:40775 returned 0
> > > > > sys_mkdir root:40775 returned 0
> > > > > sys_mkdir lib:40775 returned 0
> > > > > sys_mkdir lib/modules:40775 returned 0
> > > > > ...
> > > > > 
> > > > > and the problem is fixed.
> > > 
> > > This patch also works for me.
> > > 
> > > > ... except that it simply confirms that something's fishy with
> > > > getname_kernel() of ->name of struct filename returned by getname(). 
> > > > IOW, I still do not understand the mechanism of breakage there.
> > > 
> > > I'm not so sure about that.  I tried to copy name to a new string in
> > > do_path_lookup and that didn't help.
> > > 
> > > Now, I've removed the
> > > 
> > >         putname(filename);
> > > 
> > > line from do_path_lookup and I don't get the panic.
> > 
> > That would indicate that somehow the refcount got unbalanced. Looking
> > more closely it seems like the various audit_*() function do take a
> > reference, but maybe that's not enough.
> 
> I'm thinking the same thing and I think the problem may be that 
> __audit_reusename() is not bumping the filename->refcnt.  Can someone who is 
> seeing this problem bump the refcnt in __audit_reusename()?
> 
>   struct filename *
>   __audit_reusename(const __user char *uptr)
>   {
>         struct audit_context *context = current->audit_context;
>         struct audit_names *n;
> 
>         list_for_each_entry(n, &context->names_list, list) {
>                 if (!n->name)
>                         continue;
>                 if (n->name->uptr == uptr) {
> +                       n->name->refcnt++;
>                         return n->name;
>                 }
>         }
>         return NULL;
>   }

That doesn't seem to help, at least in my case.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Paul Moore @ 2015-01-21 15:24 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Al Viro, Sabrina Dubroca, Guenter Roeck, Stephen Rothwell,
	linux-next, linux-kernel, linux-fsdevel, linux-audit,
	Richard Guy Briggs
In-Reply-To: <20150121144214.GA22884@ulmo.nvidia.com>

On Wednesday, January 21, 2015 03:42:16 PM Thierry Reding wrote:
> On Wed, Jan 21, 2015 at 12:05:39PM +0100, Sabrina Dubroca wrote:
> > 2015-01-21, 04:36:38 +0000, Al Viro wrote:
> > > On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
> > > > With this patch:
> > > > 
> > > > sys_mkdir .:40775 returned -17
> > > > sys_mkdir usr:40775 returned 0
> > > > sys_mkdir usr/lib:40775 returned 0
> > > > sys_mkdir usr/share:40755 returned 0
> > > > sys_mkdir usr/share/udhcpc:40755 returned 0
> > > > sys_mkdir usr/bin:40775 returned 0
> > > > sys_mkdir usr/sbin:40775 returned 0
> > > > sys_mkdir mnt:40775 returned 0
> > > > sys_mkdir proc:40775 returned 0
> > > > sys_mkdir root:40775 returned 0
> > > > sys_mkdir lib:40775 returned 0
> > > > sys_mkdir lib/modules:40775 returned 0
> > > > ...
> > > > 
> > > > and the problem is fixed.
> > 
> > This patch also works for me.
> > 
> > > ... except that it simply confirms that something's fishy with
> > > getname_kernel() of ->name of struct filename returned by getname(). 
> > > IOW, I still do not understand the mechanism of breakage there.
> > 
> > I'm not so sure about that.  I tried to copy name to a new string in
> > do_path_lookup and that didn't help.
> > 
> > Now, I've removed the
> > 
> >         putname(filename);
> > 
> > line from do_path_lookup and I don't get the panic.
> 
> That would indicate that somehow the refcount got unbalanced. Looking
> more closely it seems like the various audit_*() function do take a
> reference, but maybe that's not enough.

I'm thinking the same thing and I think the problem may be that 
__audit_reusename() is not bumping the filename->refcnt.  Can someone who is 
seeing this problem bump the refcnt in __audit_reusename()?

  struct filename *
  __audit_reusename(const __user char *uptr)
  {
        struct audit_context *context = current->audit_context;
        struct audit_names *n;

        list_for_each_entry(n, &context->names_list, list) {
                if (!n->name)
                        continue;
                if (n->name->uptr == uptr) {
+                       n->name->refcnt++;
                        return n->name;
                }
        }
        return NULL;
  }

-- 
paul moore
security @ redhat

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Paul Moore @ 2015-01-21 15:06 UTC (permalink / raw)
  To: Al Viro
  Cc: Guenter Roeck, Sabrina Dubroca, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121043637.GO29656@ZenIV.linux.org.uk>

On Wednesday, January 21, 2015 04:36:38 AM Al Viro wrote:
> Another thing I really do not understand is
> +               if (inode->i_ino) {
> +                       /* valid inode number, use that for the ...
> +                       if (n->ino != inode->i_ino ||
> +                           n->dev != inode->i_sb->s_dev)
> +                               continue;
> in __audit_inode().  We don't *have* dentries with dentry->d_inode->i_ino ==
> 0. Ever.  WTF is that about?  Paul?

Likely stupidity on my part.  It looks like a typo, that first if conditional 
should check "n->ino" instead of "inode->i_ino"; in __audit_getname() we 
record names without any inode numbers, so we need to see if this is one of 
those records.  Interesting that it passed my testing; either my testing is 
crap (always a strong possibility) or something else came to the rescue.  I'm 
still coming up to speed on the audit/VFS code ...

I'll fix that up and include in the next patchset once we resolve this issue.

-- 
paul moore
security @ redhat


^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Thierry Reding @ 2015-01-21 14:42 UTC (permalink / raw)
  To: Al Viro
  Cc: Sabrina Dubroca, Guenter Roeck, Paul Moore, Stephen Rothwell,
	linux-next, linux-kernel, linux-fsdevel, linux-audit,
	Richard Guy Briggs
In-Reply-To: <20150121110539.GA8924@kria>

[-- Attachment #1: Type: text/plain, Size: 2876 bytes --]

On Wed, Jan 21, 2015 at 12:05:39PM +0100, Sabrina Dubroca wrote:
> 2015-01-21, 04:36:38 +0000, Al Viro wrote:
> > On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
> > > With this patch:
> > > 
> > > sys_mkdir .:40775 returned -17
> > > sys_mkdir usr:40775 returned 0
> > > sys_mkdir usr/lib:40775 returned 0
> > > sys_mkdir usr/share:40755 returned 0
> > > sys_mkdir usr/share/udhcpc:40755 returned 0
> > > sys_mkdir usr/bin:40775 returned 0
> > > sys_mkdir usr/sbin:40775 returned 0
> > > sys_mkdir mnt:40775 returned 0
> > > sys_mkdir proc:40775 returned 0
> > > sys_mkdir root:40775 returned 0
> > > sys_mkdir lib:40775 returned 0
> > > sys_mkdir lib/modules:40775 returned 0
> > > ...
> > > 
> > > and the problem is fixed.
> 
> This patch also works for me.
> 
> 
> > ... except that it simply confirms that something's fishy with getname_kernel()
> > of ->name of struct filename returned by getname().  IOW, I still do not
> > understand the mechanism of breakage there.
> 
> I'm not so sure about that.  I tried to copy name to a new string in
> do_path_lookup and that didn't help.
> 
> Now, I've removed the
> 
>         putname(filename);
> 
> line from do_path_lookup and I don't get the panic.

That would indicate that somehow the refcount got unbalanced. Looking
more closely it seems like the various audit_*() function do take a
reference, but maybe that's not enough.

But debugging this further I see no indication that the memory is ever
freed, or otherwise corrupted.

I did collect a bit more data, perhaps that's useful. I started seeing
this issue as well on devices that boot over NFS. After reading this
thread I also realized that another warning that I was seeing might be
related:

	[   28.261930] Warning: unable to open an initial console.

I've added a couple of printks and see that the reason for this is that
/dev/console doesn't get created. /dev however does get created.

	[   11.786627] sys_mkdir dev:40755 returned 0
	...
	[   11.978748] sys_mknod dev/console:20600 returned -2

The chain that fails turns out to be this:

	sys_mknod()
	  sys_mknodat()
	    user_path_create()
	      kern_path_create()
	        do_path_lookup()
	          filename_lookup()
	            path_lookupat()
	              path_init()
	                link_path_walk()
	                  walk_component()

walk_components() ends up calling lookup_slow() and the result is that
inode == NULL and d_is_negative(path->dentry) returns true, therefore
causing -ENOENT to be returned.

I tried to figure out why inode would be NULL at that point or why
d_is_negative() returned true, but I ended up getting completely lost,
so I thought it best to report my findings before I confuse everything.

Is there anything else I can investigate to track this down?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/5] fs: create proper filename objects using getname_kernel()
From: Paul Moore @ 2015-01-21 14:29 UTC (permalink / raw)
  To: Sasha Levin; +Cc: linux-fsdevel, linux-audit, viro, linux-kernel
In-Reply-To: <54BF21AA.5010601@oracle.com>

On Tuesday, January 20, 2015 10:48:58 PM Sasha Levin wrote:
> On 01/19/2015 03:08 PM, Paul Moore wrote:
> > There are several areas in the kernel that create temporary filename
> > 
> > objects using the following pattern:
> > 	int func(const char *name)
> > 	{
> > 	
> > 		struct filename *file = { .name = name };
> > 		...
> > 		return 0;
> > 	
> > 	}
> > 
> > ... which for the most part works okay, but it causes havoc within the
> > audit subsystem as the filename object does not persist beyond the
> > lifetime of the function.  This patch converts all of these temporary
> > filename objects into proper filename objects using getname_kernel()
> > and putname() which ensure that the filename object persists until the
> > audit subsystem is finished with it.
> 
> Hi Paul,
> 
> With this patch (bisected) my vm fails to boot with a virtio-9p rootfs:
> 
> [   27.313687] fa00         2097152 vda  driver: virtio_blk
> [   27.314218] 103:00000       8192 sda  driver: sd
> [   27.314714] DEBUG_BLOCK_EXT_DEVT is enabled, you need to specify explicit
> textual name for "root=" boot option. [   27.315963] Kernel panic - not
> syncing: VFS: Unable to mount root fs on unknown-block(0,0) [   27.316885]
> CPU: 29 PID: 1 Comm: swapper/0 Not tainted
> 3.19.0-rc5-next-20150120-sasha-00053-gb2e3c55-dirty #1772 [   27.316885] 
> ffffffffffffffff ffff88005cb8bd28 ffffffffb14522b5 000000000000004e [  
> 27.316885]  ffffffffb26d4950 ffff88005cb8bda8 ffffffffb144c298
> ffff88005cb8bd48 [   27.316885]  ffffffff00000010 ffff88005cb8bdb8
> ffff88005cb8bd58 fffffffffffffffe [   27.316885] Call Trace:
> [   27.316885]  [<ffffffffb14522b5>] dump_stack+0x4f/0x7b
> [   27.316885]  [<ffffffffb144c298>] panic+0xd2/0x216
> [   27.316885]  [<ffffffffb431558b>] mount_block_root+0x18b/0x244
> [   27.316885]  [<ffffffffb43158ae>] mount_root+0x128/0x133
> [   27.316885]  [<ffffffffb4315a1b>] prepare_namespace+0x162/0x19b
> [   27.316885]  [<ffffffffb4315223>] kernel_init_freeable+0x285/0x29a
> [   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
> [   27.316885]  [<ffffffffb14472be>] kernel_init+0xe/0xf0
> [   27.316885]  [<ffffffffb148007c>] ret_from_fork+0x7c/0xb0
> [   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
> [   27.316885] Dumping ftrace buffer:
> [   27.316885]    (ftrace buffer empty)
> [   27.316885] Kernel Offset: 0x2d000000 from 0xffffffff81000000 (relocation
> range: 0xffffffff80000000-0xffffffffbfffffff) [   27.316885] Rebooting in 1
> seconds..
> 
> virtio-9p is not even listed as an option now.

Thanks for reporting the problem, we're currently working on a fix:

 * https://lkml.org/lkml/2015/1/20/710

-- 
paul moore
security @ redhat

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21 13:32 UTC (permalink / raw)
  To: Sabrina Dubroca, Al Viro
  Cc: Paul Moore, Stephen Rothwell, linux-next, linux-kernel,
	linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121110539.GA8924@kria>

On 01/21/2015 03:05 AM, Sabrina Dubroca wrote:
> 2015-01-21, 04:36:38 +0000, Al Viro wrote:
>> On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
>>> With this patch:
>>>
>>> sys_mkdir .:40775 returned -17
>>> sys_mkdir usr:40775 returned 0
>>> sys_mkdir usr/lib:40775 returned 0
>>> sys_mkdir usr/share:40755 returned 0
>>> sys_mkdir usr/share/udhcpc:40755 returned 0
>>> sys_mkdir usr/bin:40775 returned 0
>>> sys_mkdir usr/sbin:40775 returned 0
>>> sys_mkdir mnt:40775 returned 0
>>> sys_mkdir proc:40775 returned 0
>>> sys_mkdir root:40775 returned 0
>>> sys_mkdir lib:40775 returned 0
>>> sys_mkdir lib/modules:40775 returned 0
>>> ...
>>>
>>> and the problem is fixed.
>
> This patch also works for me.
>
>
>> ... except that it simply confirms that something's fishy with getname_kernel()
>> of ->name of struct filename returned by getname().  IOW, I still do not
>> understand the mechanism of breakage there.
>
> I'm not so sure about that.  I tried to copy name to a new string in
> do_path_lookup and that didn't help.
>
> Now, I've removed the
>
>          putname(filename);
>
> line from do_path_lookup and I don't get the panic.
>
>
> And BTW, I added Guenter's debugging to init/initramfs.c and got:
> sys_mkdir dev:40755 returned 0
> sys_mkdir root:40700 returned 0
>
> even if it ends up panic'ing.
>
Another data point (though I have no idea if it is useful or what it means):

In the working case, path_init sets nd->flags to 0x50 or 0x51.
In the non-working case (ie for all files with a '/' in the name),
it sets nd->flags to 0x10 or 0x11, even though it is always called
with the LOOKUP_RCU bit set in flags.

Guenter


^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Sabrina Dubroca @ 2015-01-21 11:05 UTC (permalink / raw)
  To: Al Viro
  Cc: Guenter Roeck, Paul Moore, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121043637.GO29656@ZenIV.linux.org.uk>

2015-01-21, 04:36:38 +0000, Al Viro wrote:
> On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
> > With this patch:
> > 
> > sys_mkdir .:40775 returned -17
> > sys_mkdir usr:40775 returned 0
> > sys_mkdir usr/lib:40775 returned 0
> > sys_mkdir usr/share:40755 returned 0
> > sys_mkdir usr/share/udhcpc:40755 returned 0
> > sys_mkdir usr/bin:40775 returned 0
> > sys_mkdir usr/sbin:40775 returned 0
> > sys_mkdir mnt:40775 returned 0
> > sys_mkdir proc:40775 returned 0
> > sys_mkdir root:40775 returned 0
> > sys_mkdir lib:40775 returned 0
> > sys_mkdir lib/modules:40775 returned 0
> > ...
> > 
> > and the problem is fixed.

This patch also works for me.


> ... except that it simply confirms that something's fishy with getname_kernel()
> of ->name of struct filename returned by getname().  IOW, I still do not
> understand the mechanism of breakage there.

I'm not so sure about that.  I tried to copy name to a new string in
do_path_lookup and that didn't help.

Now, I've removed the

        putname(filename);

line from do_path_lookup and I don't get the panic.


And BTW, I added Guenter's debugging to init/initramfs.c and got:
sys_mkdir dev:40755 returned 0
sys_mkdir root:40700 returned 0

even if it ends up panic'ing.

-- 
Sabrina

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-21  4:36 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Paul Moore, Sabrina Dubroca, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <54BF2496.4000807@roeck-us.net>

On Tue, Jan 20, 2015 at 08:01:26PM -0800, Guenter Roeck wrote:
> With this patch:
> 
> sys_mkdir .:40775 returned -17
> sys_mkdir usr:40775 returned 0
> sys_mkdir usr/lib:40775 returned 0
> sys_mkdir usr/share:40755 returned 0
> sys_mkdir usr/share/udhcpc:40755 returned 0
> sys_mkdir usr/bin:40775 returned 0
> sys_mkdir usr/sbin:40775 returned 0
> sys_mkdir mnt:40775 returned 0
> sys_mkdir proc:40775 returned 0
> sys_mkdir root:40775 returned 0
> sys_mkdir lib:40775 returned 0
> sys_mkdir lib/modules:40775 returned 0
> ...
> 
> and the problem is fixed.

... except that it simply confirms that something's fishy with getname_kernel()
of ->name of struct filename returned by getname().  IOW, I still do not
understand the mechanism of breakage there.

Another thing I really do not understand is
+               if (inode->i_ino) {
+                       /* valid inode number, use that for the comparison */
+                       if (n->ino != inode->i_ino ||
+                           n->dev != inode->i_sb->s_dev)
+                               continue;
in __audit_inode().  We don't *have* dentries with dentry->d_inode->i_ino == 0.
Ever.  WTF is that about?  Paul?

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21  4:01 UTC (permalink / raw)
  To: Al Viro
  Cc: Paul Moore, Sabrina Dubroca, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121033600.GN29656@ZenIV.linux.org.uk>

On 01/20/2015 07:36 PM, Al Viro wrote:
> On Tue, Jan 20, 2015 at 06:44:34PM -0800, Guenter Roeck wrote:
>>> The shit hits the fan earlier - when we end up missing /dev.  There are
>>> two places where it could've been created (depending on CONFIG_BLK_DEV_INITRD);
>>> 	sys_mkdir(collected, mode);
>>> in init/initramfs.c (line 353 in linux-next) and
>>>          err = sys_mkdir((const char __user __force *) "/dev", 0755);
>>> in init/noinitramfs.c (line 32).  The latter would've screamed on failure;
>>> could you printk of collected (%s), mode (%o) and return value (%d) in the
>>> former and see what happens?
>>>
>>
>> sys_mkdir .:40775 returned -17
>> sys_mkdir usr:40775 returned 0
>> sys_mkdir usr/lib:40775 returned -2
>> sys_mkdir usr/share:40755 returned -2
>> sys_mkdir usr/share/udhcpc:40755 returned -2
>> sys_mkdir usr/bin:40775 returned -2
>> sys_mkdir usr/sbin:40775 returned -2
>> sys_mkdir mnt:40775 returned 0
>> sys_mkdir proc:40775 returned 0
>> sys_mkdir root:40775 returned 0
>> sys_mkdir lib:40775 returned 0
>> sys_mkdir lib/modules:40775 returned -2
>> sys_mkdir lib/modules/3.9.2:40775 returned -2
>> sys_mkdir lib/modules/3.9.2/kernel:40775 returned -2
>>
>> with
>> 	int err = sys_mkdir(collected, mode);
>> 	pr_info("sys_mkdir %s:%o returned %d\n", collected, mode, err);
>> added in init/initramfs.c.
>
> Just what is lib/modules/3.9.2 doing there?  In any case, I think I have at

Artifact from when I created the root file system (which apparently was with
3.9.2). It is irrelevant for my testing, at least so far, so I never
bothered fixing it.

> least a plausible direction for digging.  Look:
>

> diff --git a/fs/namei.c b/fs/namei.c
> index 323957f..c7d107c 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c

With this patch:

sys_mkdir .:40775 returned -17
sys_mkdir usr:40775 returned 0
sys_mkdir usr/lib:40775 returned 0
sys_mkdir usr/share:40755 returned 0
sys_mkdir usr/share/udhcpc:40755 returned 0
sys_mkdir usr/bin:40775 returned 0
sys_mkdir usr/sbin:40775 returned 0
sys_mkdir mnt:40775 returned 0
sys_mkdir proc:40775 returned 0
sys_mkdir root:40775 returned 0
sys_mkdir lib:40775 returned 0
sys_mkdir lib/modules:40775 returned 0
...

and the problem is fixed.

Guenter

^ permalink raw reply

* Re: [PATCH 2/5] fs: create proper filename objects using getname_kernel()
From: Sasha Levin @ 2015-01-21  3:48 UTC (permalink / raw)
  To: Paul Moore, linux-fsdevel, linux-audit; +Cc: viro, linux-kernel
In-Reply-To: <20150119200808.29706.73419.stgit@localhost>

On 01/19/2015 03:08 PM, Paul Moore wrote:
> There are several areas in the kernel that create temporary filename
> objects using the following pattern:
> 
> 	int func(const char *name)
> 	{
> 		struct filename *file = { .name = name };
> 		...
> 		return 0;
> 	}
> 
> ... which for the most part works okay, but it causes havoc within the
> audit subsystem as the filename object does not persist beyond the
> lifetime of the function.  This patch converts all of these temporary
> filename objects into proper filename objects using getname_kernel()
> and putname() which ensure that the filename object persists until the
> audit subsystem is finished with it.

Hi Paul,

With this patch (bisected) my vm fails to boot with a virtio-9p rootfs:

[   27.313687] fa00         2097152 vda  driver: virtio_blk
[   27.314218] 103:00000       8192 sda  driver: sd
[   27.314714] DEBUG_BLOCK_EXT_DEVT is enabled, you need to specify explicit textual name for "root=" boot option.
[   27.315963] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
[   27.316885] CPU: 29 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc5-next-20150120-sasha-00053-gb2e3c55-dirty #1772
[   27.316885]  ffffffffffffffff ffff88005cb8bd28 ffffffffb14522b5 000000000000004e
[   27.316885]  ffffffffb26d4950 ffff88005cb8bda8 ffffffffb144c298 ffff88005cb8bd48
[   27.316885]  ffffffff00000010 ffff88005cb8bdb8 ffff88005cb8bd58 fffffffffffffffe
[   27.316885] Call Trace:
[   27.316885]  [<ffffffffb14522b5>] dump_stack+0x4f/0x7b
[   27.316885]  [<ffffffffb144c298>] panic+0xd2/0x216
[   27.316885]  [<ffffffffb431558b>] mount_block_root+0x18b/0x244
[   27.316885]  [<ffffffffb43158ae>] mount_root+0x128/0x133
[   27.316885]  [<ffffffffb4315a1b>] prepare_namespace+0x162/0x19b
[   27.316885]  [<ffffffffb4315223>] kernel_init_freeable+0x285/0x29a
[   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
[   27.316885]  [<ffffffffb14472be>] kernel_init+0xe/0xf0
[   27.316885]  [<ffffffffb148007c>] ret_from_fork+0x7c/0xb0
[   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
[   27.316885] Dumping ftrace buffer:
[   27.316885]    (ftrace buffer empty)
[   27.316885] Kernel Offset: 0x2d000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[   27.316885] Rebooting in 1 seconds..

virtio-9p is not even listed as an option now.

Thanks,
Sasha

^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-21  3:36 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Paul Moore, Sabrina Dubroca, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <54BF1292.2080902@roeck-us.net>

On Tue, Jan 20, 2015 at 06:44:34PM -0800, Guenter Roeck wrote:
> >The shit hits the fan earlier - when we end up missing /dev.  There are
> >two places where it could've been created (depending on CONFIG_BLK_DEV_INITRD);
> >	sys_mkdir(collected, mode);
> >in init/initramfs.c (line 353 in linux-next) and
> >         err = sys_mkdir((const char __user __force *) "/dev", 0755);
> >in init/noinitramfs.c (line 32).  The latter would've screamed on failure;
> >could you printk of collected (%s), mode (%o) and return value (%d) in the
> >former and see what happens?
> >
> 
> sys_mkdir .:40775 returned -17
> sys_mkdir usr:40775 returned 0
> sys_mkdir usr/lib:40775 returned -2
> sys_mkdir usr/share:40755 returned -2
> sys_mkdir usr/share/udhcpc:40755 returned -2
> sys_mkdir usr/bin:40775 returned -2
> sys_mkdir usr/sbin:40775 returned -2
> sys_mkdir mnt:40775 returned 0
> sys_mkdir proc:40775 returned 0
> sys_mkdir root:40775 returned 0
> sys_mkdir lib:40775 returned 0
> sys_mkdir lib/modules:40775 returned -2
> sys_mkdir lib/modules/3.9.2:40775 returned -2
> sys_mkdir lib/modules/3.9.2/kernel:40775 returned -2
> 
> with
> 	int err = sys_mkdir(collected, mode);
> 	pr_info("sys_mkdir %s:%o returned %d\n", collected, mode, err);
> added in init/initramfs.c.

Just what is lib/modules/3.9.2 doing there?  In any case, I think I have at
least a plausible direction for digging.  Look:

struct dentry *user_path_create(int dfd, const char __user *pathname,
                                struct path *path, unsigned int lookup_flags)
{
        struct filename *tmp = getname(pathname);
        struct dentry *res;
        if (IS_ERR(tmp))
                return ERR_CAST(tmp);
        res = kern_path_create(dfd, tmp->name, path, lookup_flags);

struct dentry *kern_path_create(int dfd, const char *pathname,
                                struct path *path, unsigned int lookup_flags)
{
        struct dentry *dentry = ERR_PTR(-EEXIST);
        struct nameidata nd;
        int err2;
        int error;
        bool is_dir = (lookup_flags & LOOKUP_DIRECTORY);

        /*
         * Note that only LOOKUP_REVAL and LOOKUP_DIRECTORY matter here. Any
         * other flags passed in are ignored!
         */
        lookup_flags &= LOOKUP_REVAL;

        error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);

static int do_path_lookup(int dfd, const char *name,
                                unsigned int flags, struct nameidata *nd)
{
        int retval;
        struct filename *filename;

        filename = getname_kernel(name);
        if (unlikely(IS_ERR(filename)))
                return PTR_ERR(filename);
        retval = filename_lookup(dfd, filename, flags, nd);

and we have done getname_kernel() on the name->name of result of getname().
At the very least, it's pointless - we already *have* struct filename for
that sucker.  Now, it shouldn't have screwed the things up - it would better
not, anyway, since we might legitimately have two identical pathname
among the syscall arguments.  However, let's see if this (on top of
linux-next, in addition to the same printks) changes behaviour:

diff --git a/fs/namei.c b/fs/namei.c
index 323957f..c7d107c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3314,7 +3314,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 	return file;
 }
 
-struct dentry *kern_path_create(int dfd, const char *pathname,
+static struct dentry *__kern_path_create(int dfd, struct filename *name,
 				struct path *path, unsigned int lookup_flags)
 {
 	struct dentry *dentry = ERR_PTR(-EEXIST);
@@ -3329,7 +3329,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname,
 	 */
 	lookup_flags &= LOOKUP_REVAL;
 
-	error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
+	error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
 	if (error)
 		return ERR_PTR(error);
 
@@ -3383,6 +3383,19 @@ out:
 	path_put(&nd.path);
 	return dentry;
 }
+
+struct dentry *kern_path_create(int dfd, const char *pathname,
+				struct path *path, unsigned int lookup_flags)
+{
+	struct filename *filename = getname_kernel(pathname);
+	struct dentry *res = ERR_CAST(filename);
+
+	if (!IS_ERR(filename)) {
+		res = __kern_path_create(dfd, filename, path, lookup_flags);
+		putname(filename);
+	}
+	return res;
+}
 EXPORT_SYMBOL(kern_path_create);
 
 void done_path_create(struct path *path, struct dentry *dentry)
@@ -3401,7 +3414,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname,
 	struct dentry *res;
 	if (IS_ERR(tmp))
 		return ERR_CAST(tmp);
-	res = kern_path_create(dfd, tmp->name, path, lookup_flags);
+	res = __kern_path_create(dfd, tmp, path, lookup_flags);
 	putname(tmp);
 	return res;
 }

^ permalink raw reply related

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21  2:44 UTC (permalink / raw)
  To: Al Viro, Paul Moore
  Cc: Sabrina Dubroca, Stephen Rothwell, linux-next, linux-kernel,
	linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121004117.GM29656@ZenIV.linux.org.uk>

On 01/20/2015 04:41 PM, Al Viro wrote:

> The shit hits the fan earlier - when we end up missing /dev.  There are
> two places where it could've been created (depending on CONFIG_BLK_DEV_INITRD);
> 	sys_mkdir(collected, mode);
> in init/initramfs.c (line 353 in linux-next) and
>          err = sys_mkdir((const char __user __force *) "/dev", 0755);
> in init/noinitramfs.c (line 32).  The latter would've screamed on failure;
> could you printk of collected (%s), mode (%o) and return value (%d) in the
> former and see what happens?
>

sys_mkdir .:40775 returned -17
sys_mkdir usr:40775 returned 0
sys_mkdir usr/lib:40775 returned -2
sys_mkdir usr/share:40755 returned -2
sys_mkdir usr/share/udhcpc:40755 returned -2
sys_mkdir usr/bin:40775 returned -2
sys_mkdir usr/sbin:40775 returned -2
sys_mkdir mnt:40775 returned 0
sys_mkdir proc:40775 returned 0
sys_mkdir root:40775 returned 0
sys_mkdir lib:40775 returned 0
sys_mkdir lib/modules:40775 returned -2
sys_mkdir lib/modules/3.9.2:40775 returned -2
sys_mkdir lib/modules/3.9.2/kernel:40775 returned -2

with
	int err = sys_mkdir(collected, mode);
	pr_info("sys_mkdir %s:%o returned %d\n", collected, mode, err);
added in init/initramfs.c.

Guenter


^ permalink raw reply

* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-21  0:41 UTC (permalink / raw)
  To: Paul Moore
  Cc: Sabrina Dubroca, Guenter Roeck, Stephen Rothwell, linux-next,
	linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <1528536.jyty2pu4Mz@sifl>

On Tue, Jan 20, 2015 at 07:14:46PM -0500, Paul Moore wrote:
> > > with the patch applied (+panic)
> > > 
> > > 
> > > and:
> > > 
> > > stat("/dev/root") -> 0
> > > stat("dev") -> 0
> > > with the old version of do_path_lookup.
> > 
> > Wait a minute ... at this early stage of boot, I'm pretty sure we don't have
> > a valid current->audit_context since we haven't fork'd anything.  If the
> > audit context was non-NULL garbage that might explain the panic ...

Not really - the panic is predicatable result of not finding /dev/root (hell,
not even /dev there!) in mount_block_root().
 
> Could you try initializing the init_task's audit_context to NULL in the 
> INIT_TASK macro in include/linux/init_task.h?

Missing fields in struct initializer are treated as zeroes.

The shit hits the fan earlier - when we end up missing /dev.  There are
two places where it could've been created (depending on CONFIG_BLK_DEV_INITRD);
	sys_mkdir(collected, mode);
in init/initramfs.c (line 353 in linux-next) and
        err = sys_mkdir((const char __user __force *) "/dev", 0755);
in init/noinitramfs.c (line 32).  The latter would've screamed on failure;
could you printk of collected (%s), mode (%o) and return value (%d) in the
former and see what happens?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox