2.6.37-next - kernel BUG at fs/dcache.c:1363

2.6.37-next - kernel BUG at fs/dcache.c:1363

[Valdis.Kletnieks]

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

Saw this crash on a linux-next pulled yesterday at 2PM EST, kernel dies very
early (looks like first time it touches configfs for anything - trying to boot
with netconsole enabled caused it to die even faster).  I can bisect this if
it doesn't immediately ring a bell...

It dies here:

void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
{
        BUG_ON(dentry->d_op);

Am guessing configfs passed in a dentry that wasn't filled in enough.

(hand-transcribed from a crappy cellphone pic)

kernel BUG at fs/dcache.c:1363
invalid opcode: 0000 [#1] PREEMT SMP
last sysfs file:
CPU 0
Modules linked in:

Pid: 1, comm: swapper Not tainted 2.6.37-rc8-next-2011 (edge of pic)
...

configfs_attach_item.clone.14+0x11d/0x254
configfs_attach_group.clone.15+0x1c/0x196
? _raw_spinlock_unlock+0x5c/0x69
configfs_register_subsystem_0xce/0x144
? init_netconsole+0x0/0x21f
init_netconsole+0x10d/0x21f
? init_netconsole+0x0/0x21f
do_one_initcall+0x52/0x12f
kernel_init+0x162/0x1e7
kernel_thread_helper+0x4/0x10
? finish_task_switch_0x3f/0xe3
? restore_args+0x0/0x30
? kernel_init+0x0/0x1e7
? kernel_thread_helper+0x0/0x10

I d_net_d_op+0x38/0xb0



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

Re: 2.6.37-next - kernel BUG at fs/dcache.c:1363

[Nick Piggin]

On Thu, Jan 6, 2011 at 4:15 AM,  <Valdis.Kletnieks@vt.edu> wrote:
> Saw this crash on a linux-next pulled yesterday at 2PM EST, kernel dies very
> early (looks like first time it touches configfs for anything - trying to boot
> with netconsole enabled caused it to die even faster).  I can bisect this if
> it doesn't immediately ring a bell...

Thanks, bah configfs isn't widely used.


> It dies here:
>
> void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
> {
>        BUG_ON(dentry->d_op);
>
> Am guessing configfs passed in a dentry that wasn't filled in enough.
>
> (hand-transcribed from a crappy cellphone pic)
>
> kernel BUG at fs/dcache.c:1363

Thanks. It actually passed in a dentry that appears to have already been
used for something. This is not exactly a nice thing for a filesystem to do
and probably indicates an underlying bug anyway (or at least something
the vfs doesn't guarantee the safety of).

Taking a look now.

Thanks,
Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 2.6.37-next - kernel BUG at fs/dcache.c:1363

[Nick Piggin]

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

On Thu, Jan 6, 2011 at 9:44 PM, Nick Piggin <npiggin@gmail.com> wrote:
> On Thu, Jan 6, 2011 at 4:15 AM,  <Valdis.Kletnieks@vt.edu> wrote:
>> Saw this crash on a linux-next pulled yesterday at 2PM EST, kernel dies very
>> early (looks like first time it touches configfs for anything - trying to boot
>> with netconsole enabled caused it to die even faster).  I can bisect this if
>> it doesn't immediately ring a bell...
>
> Thanks, bah configfs isn't widely used.
>
>
>> It dies here:
>>
>> void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
>> {
>>        BUG_ON(dentry->d_op);
>>
>> Am guessing configfs passed in a dentry that wasn't filled in enough.
>>
>> (hand-transcribed from a crappy cellphone pic)
>>
>> kernel BUG at fs/dcache.c:1363
>
> Thanks. It actually passed in a dentry that appears to have already been
> used for something. This is not exactly a nice thing for a filesystem to do
> and probably indicates an underlying bug anyway (or at least something
> the vfs doesn't guarantee the safety of).
>
> Taking a look now.

This patch fixes it here

[-- Attachment #2: dentry-debug.patch --]
[-- Type: application/octet-stream, Size: 2449 bytes --]

config fs: avoid switching ->d_op on live dentry

Switching d_op on a live dentry is racy in general, so avoid it. In this case
it is a negative dentry, which is safer, but there are still concurrent ops
which may be called on d_op in that case (eg. d_revalidate). So in general
a filesystem may not do this. Fix configfs so as not to do this.

[to be backmerged or replaced with Al Viro's alternative]

Signed-off-by: Nick Piggin <npiggin@kernel.dk>

Index: linux-2.6/fs/configfs/dir.c
===================================================================
--- linux-2.6.orig/fs/configfs/dir.c	2011-01-06 21:55:58.000000000 +1100
+++ linux-2.6/fs/configfs/dir.c	2011-01-07 00:06:43.000000000 +1100
@@ -232,10 +232,8 @@ int configfs_make_dirent(struct configfs
 
 	sd->s_mode = mode;
 	sd->s_dentry = dentry;
-	if (dentry) {
+	if (dentry)
 		dentry->d_fsdata = configfs_get(sd);
-		d_set_d_op(dentry, &configfs_dentry_ops);
-	}
 
 	return 0;
 }
@@ -278,7 +276,6 @@ static int create_dir(struct config_item
 		error = configfs_create(d, mode, init_dir);
 		if (!error) {
 			inc_nlink(p->d_inode);
-			d_set_d_op((d), &configfs_dentry_ops);
 		} else {
 			struct configfs_dirent *sd = d->d_fsdata;
 			if (sd) {
@@ -371,9 +368,7 @@ int configfs_create_link(struct configfs
 				   CONFIGFS_ITEM_LINK);
 	if (!err) {
 		err = configfs_create(dentry, mode, init_symlink);
-		if (!err)
-			d_set_d_op(dentry, &configfs_dentry_ops);
-		else {
+		if (err) {
 			struct configfs_dirent *sd = dentry->d_fsdata;
 			if (sd) {
 				spin_lock(&configfs_dirent_lock);
@@ -492,7 +487,11 @@ static struct dentry * configfs_lookup(s
 		 * If it doesn't exist and it isn't a NOT_PINNED item,
 		 * it must be negative.
 		 */
-		return simple_lookup(dir, dentry, nd);
+		if (dentry->d_name.len > NAME_MAX)
+			return ERR_PTR(-ENAMETOOLONG);
+		d_set_d_op(dentry, &configfs_dentry_ops);
+		d_add(dentry, NULL);
+		return NULL;
 	}
 
 out:
@@ -684,6 +683,7 @@ static int create_default_group(struct c
 	ret = -ENOMEM;
 	child = d_alloc(parent, &name);
 	if (child) {
+		d_set_d_op(child, &configfs_dentry_ops);
 		d_add(child, NULL);
 
 		ret = configfs_attach_group(&parent_group->cg_item,
@@ -1681,6 +1681,7 @@ int configfs_register_subsystem(struct c
 	err = -ENOMEM;
 	dentry = d_alloc(configfs_sb->s_root, &name);
 	if (dentry) {
+		d_set_d_op(dentry, &configfs_dentry_ops);
 		d_add(dentry, NULL);
 
 		err = configfs_attach_group(sd->s_element, &group->cg_item,

Re: 2.6.37-next - kernel BUG at fs/dcache.c:1363

[Valdis.Kletnieks]

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

On Fri, 07 Jan 2011 00:12:09 +1100, Nick Piggin said:

> > Thanks. It actually passed in a dentry that appears to have already been
> > used for something. This is not exactly a nice thing for a filesystem to do
> > and probably indicates an underlying bug anyway (or at least something
> > the vfs doesn't guarantee the safety of).
> >
> > Taking a look now.
>
> This patch fixes it here

> config fs: avoid switching ->d_op on live dentry

> Switching d_op on a live dentry is racy in general, so avoid it. In this case
> it is a negative dentry, which is safer, but there are still concurrent ops
> which may be called on d_op in that case (eg. d_revalidate). So in general
> a filesystem may not do this. Fix configfs so as not to do this.

Confirming this patch fixes the crash I was seeing, so now I'm off to try to
finish bisecting my way through the other issue I was seeing...

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