* [nfs] Oops during call nfs_mountpoint_inode_operations->lookup() method
@ 2011-01-17 16:55 Vitaliy Gusev
2011-01-17 17:04 ` Vitaliy Gusev
0 siblings, 1 reply; 4+ messages in thread
From: Vitaliy Gusev @ 2011-01-17 16:55 UTC (permalink / raw)
To: Trond.Myklebust; +Cc: linux-fsdevel, akpm
Hello!
I had tried mount NFS4.1 server and caught Oops in NFS4.1 client.
After some investigation I saw that NFSv4 client is also buggy.
Simple NFSv4 server modification can raise Oops in any linux NFSv4 client since
commit c02d7adf8c5429727a98bad1d039bccad4c61c50 .
The problem is a dereference and call i_op->lookup method, but
nfs_mountpoint_inode_operations
doesn't have that.
Oops in NFSv4 client:
[ 4022.269400] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 4022.269406] IP: [<(null)>] (null)
[ 4022.269442] PGD 0
[ 4022.269446] Oops: 0010 [#1] SMP
[ 4022.269448] last sysfs file: /sys/devices/virtual/bdi/0:21/uevent
[ 4022.269458] CPU 0
[ 4022.269459] Modules linked in: nfs_layout_nfsv41_files nfs lockd
fscache nfs_acl auth_rpcgss fuse sunrpc ip6t_REJECT nf_conntrack_ipv6
ip6table_filter ip6_tables ipv6 uinput vmw_balloon microcode ppdev
snd_ens1371 gameport snd_rawmidi snd_ac97_codec ac97_bus snd_seq
snd_seq_device parport_pc parport snd_pcm snd_timer snd e1000
soundcore snd_page_alloc i2c_piix4 shpchp i2c_core mptspi mptscsih
mptbase scsi_transport_spi [last unloaded: speedstep_lib]
[ 4022.269555]
[ 4022.269561] Pid: 2581, comm: mount.nfs4 Not tainted
2.6.36-1.pnfs_all_2010_11_03.fc15.x86_64 #1 440BX Desktop Reference
Platform/VMware Virtual Platform
[ 4022.269564] RIP: 0010:[<0000000000000000>] [<(null)>] (null)
[ 4022.269567] RSP: 0018:ffff880032469bc0 EFLAGS: 00010282
[ 4022.269569] RAX: ffffffffa02c7a60 RBX: ffff8800333cf858 RCX: 000000000000cb90
[ 4022.269571] RDX: ffff8800323ef5a8 RSI: ffff8800333cf858 RDI: ffff880016242268
[ 4022.269573] RBP: ffff880032469be8 R08: ffff880016242358 R09: ffffffff811406cd
[ 4022.269575] R10: ffffffff811406cd R11: ffffffff81a42798 R12: ffff880016242268
[ 4022.269577] R13: ffff880032469c78 R14: ffff880016242358 R15: ffff8800333cf5c8
[ 4022.269591] FS: 00007f61dcb96720(0000) GS:ffff880004400000(0000)
knlGS:0000000000000000
[ 4022.269594] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 4022.269596] CR2: 0000000000000000 CR3: 000000003210a000 CR4: 00000000000006f0
[ 4022.269630] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 4022.269649] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 4022.269652] Process mount.nfs4 (pid: 2581, threadinfo
ffff880032468000, task ffff88003106c880)
[ 4022.269654] Stack:
[ 4022.269655] ffffffff8113740c ffff880032469be8 ffff8800323ef5a8
ffff8800323ef5a8
[ 4022.269659] <0> ffff880032469c68 ffff880032469c38 ffffffff8113753c
ffff880016242268
[ 4022.269664] <0> ffff88002f708500 ffff880032469c18 ffff8800323ef5a8
ffff8800196c18f8
[ 4022.269668] Call Trace:
[ 4022.269710] [<ffffffff8113740c>] ? d_alloc_and_lookup+0x4c/0x74
[ 4022.269715] [<ffffffff8113753c>] do_lookup+0xbb/0x10d
[ 4022.269731] [<ffffffff8113882a>] link_path_walk+0x1bf/0x47c
[ 4022.269753] [<ffffffff8107d99d>] ? trace_hardirqs_off+0xd/0xf
[ 4022.269756] [<ffffffff8107dddb>] ? lock_release_holdtime+0x54/0x5b
[ 4022.269760] [<ffffffff81138bdc>] path_walk+0x4f/0x9f
[ 4022.269763] [<ffffffff81138c90>] vfs_path_lookup+0x64/0xaa
[ 4022.269817] [<ffffffffa029b4e4>] nfs_follow_remote_path+0x167/0x2a0 [nfs]
[ 4022.269828] [<ffffffffa029c575>] nfs4_try_mount.clone.11+0x78/0xb2 [nfs]
[ 4022.269838] [<ffffffffa029c60c>] nfs4_get_sb+0x5d/0xd3 [nfs]
[ 4022.269841] [<ffffffff81130823>] vfs_kern_mount+0xad/0x1ac
[ 4022.269844] [<ffffffff8113098a>] do_kern_mount+0x4d/0xef
[ 4022.269851] [<ffffffff81147756>] do_mount+0x1df/0x239
[ 4022.269854] [<ffffffff81147890>] sys_mount+0x88/0xc2
[ 4022.269884] [<ffffffff81009d72>] system_call_fastpath+0x16/0x1b
[ 4022.269887] Code: Bad RIP value.
[ 4022.269905] RIP [<(null)>] (null)
[ 4022.269907] RSP <ffff880032469bc0>
[ 4022.269908] CR2: 0000000000000000
[ 4022.269925] ---[ end trace 40f778ff7786c338 ]---
------------
TEST NFSv4 server modification:
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4aa6278..181048e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2042,9 +2042,11 @@ nfsd4_encode_fattr(struct svc_fh *fhp, struct
svc_export *exp,
WRITE64(NFS4_REFERRAL_FSID_MAJOR);
WRITE64(NFS4_REFERRAL_FSID_MINOR);
} else switch(fsid_source(fhp)) {
+ static unsigned key = 0;
case FSIDSOURCE_FSID:
- WRITE64((u64)exp->ex_fsid);
+ WRITE64((u64)exp->ex_fsid + key);
WRITE64((u64)0);
+ key++;
break;
case FSIDSOURCE_DEV:
WRITE32(0);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [nfs] Oops during call nfs_mountpoint_inode_operations->lookup() method
2011-01-17 16:55 [nfs] Oops during call nfs_mountpoint_inode_operations->lookup() method Vitaliy Gusev
@ 2011-01-17 17:04 ` Vitaliy Gusev
2011-01-17 22:18 ` Al Viro
0 siblings, 1 reply; 4+ messages in thread
From: Vitaliy Gusev @ 2011-01-17 17:04 UTC (permalink / raw)
To: Trond.Myklebust; +Cc: linux-fsdevel, akpm
nfs: Add .lookup methods to nfs_mountpoint_inode_operations
NFSv4 client code in some cases can call generic vfs_path_lookup() during
mount(2). Inside this generic function .lookup method should be called. But
nfs_mountpoint_inode_operations and nfs_referral_inode_operations
don't have .lookup method that causes Oops:
d_alloc_and_lookup+0x4c/0x74
do_lookup+0x1e3/0x280
link_path_walk+0x12e/0xab0
nfs4_remote_get_sb+0x56/0x2c0 [nfs]
path_walk+0x67/0xe0
vfs_path_lookup+0x8e/0x100
nfs_follow_remote_path+0x16f/0x3e0 [nfs]
nfs4_try_mount+0x6f/0xd0 [nfs]
nfs_get_sb+0x269/0x400 [nfs]
vfs_kern_mount+0x8a/0x1f0
do_kern_mount+0x52/0x130
do_mount+0x20a/0x260
sys_mount+0x90/0xe0
system_call_fastpath+0x16/0x1b
Signed-off-by: Gusev Vitaliy <gusev.vitaliy@gmail.com>
---
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 662df2a..9d60b29 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -110,7 +110,6 @@ const struct inode_operations nfs3_dir_inode_operations = {
#ifdef CONFIG_NFS_V4
-static struct dentry *nfs_atomic_lookup(struct inode *, struct dentry
*, struct nameidata *);
static int nfs_open_create(struct inode *dir, struct dentry *dentry,
int mode, struct nameidata *nd);
const struct inode_operations nfs4_dir_inode_operations = {
.create = nfs_open_create,
@@ -1317,7 +1316,7 @@ out:
return ret;
}
-static struct dentry *nfs_atomic_lookup(struct inode *dir, struct
dentry *dentry, struct nameidata *nd)
+struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry
*dentry, struct nameidata *nd)
{
struct nfs_open_context *ctx;
struct iattr attr;
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 0a9ea58..92c4627 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -266,6 +266,7 @@ extern struct dentry *nfs_get_root(struct
super_block *, struct nfs_fh *);
extern struct dentry *nfs4_get_root(struct super_block *, struct nfs_fh *);
extern int nfs4_get_rootfh(struct nfs_server *server, struct nfs_fh *mntfh);
+extern struct dentry *nfs_atomic_lookup(struct inode *, struct dentry
*, struct nameidata *);
#endif
/* read.c */
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index db6aa36..c5f9162 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -177,10 +177,16 @@ out_follow:
const struct inode_operations nfs_mountpoint_inode_operations = {
.follow_link = nfs_follow_mountpoint,
.getattr = nfs_getattr,
+#ifdef CONFIG_NFS_V4
+ .lookup = nfs_atomic_lookup,
+#endif
};
const struct inode_operations nfs_referral_inode_operations = {
.follow_link = nfs_follow_mountpoint,
+#ifdef CONFIG_NFS_V4
+ .lookup = nfs_atomic_lookup,
+#endif
};
static void nfs_expire_automounts(struct work_struct *work)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [nfs] Oops during call nfs_mountpoint_inode_operations->lookup() method
2011-01-17 17:04 ` Vitaliy Gusev
@ 2011-01-17 22:18 ` Al Viro
2011-01-18 22:59 ` Vitaliy Gusev
0 siblings, 1 reply; 4+ messages in thread
From: Al Viro @ 2011-01-17 22:18 UTC (permalink / raw)
To: Vitaliy Gusev; +Cc: Trond.Myklebust, linux-fsdevel, akpm
On Mon, Jan 17, 2011 at 08:04:02PM +0300, Vitaliy Gusev wrote:
> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
> index db6aa36..c5f9162 100644
> --- a/fs/nfs/namespace.c
> +++ b/fs/nfs/namespace.c
> @@ -177,10 +177,16 @@ out_follow:
> const struct inode_operations nfs_mountpoint_inode_operations = {
> .follow_link = nfs_follow_mountpoint,
> .getattr = nfs_getattr,
> +#ifdef CONFIG_NFS_V4
> + .lookup = nfs_atomic_lookup,
> +#endif
> };
>
> const struct inode_operations nfs_referral_inode_operations = {
> .follow_link = nfs_follow_mountpoint,
> +#ifdef CONFIG_NFS_V4
> + .lookup = nfs_atomic_lookup,
> +#endif
> };
That looks very wrong. You are papering over the real problem here - ask
yourself what would that ->lookup() be expected to do?
The thing is, we shouldn't end up trying to call ->lookup() for those.
That's the real issue...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [nfs] Oops during call nfs_mountpoint_inode_operations->lookup() method
2011-01-17 22:18 ` Al Viro
@ 2011-01-18 22:59 ` Vitaliy Gusev
0 siblings, 0 replies; 4+ messages in thread
From: Vitaliy Gusev @ 2011-01-18 22:59 UTC (permalink / raw)
To: Al Viro; +Cc: Trond.Myklebust, linux-fsdevel, akpm
Hello!
2011/1/18 Al Viro <viro@zeniv.linux.org.uk>:
>> const struct inode_operations nfs_referral_inode_operations = {
>> .follow_link = nfs_follow_mountpoint,
>> +#ifdef CONFIG_NFS_V4
>> + .lookup = nfs_atomic_lookup,
>> +#endif
>> };
>
> That looks very wrong. You are papering over the real problem here - ask
> yourself what would that ->lookup() be expected to do?
That ->lookup() have to search first directory name in Exported path on
NFS server. However, maybe it is not that expected...
>
> The thing is, we shouldn't end up trying to call ->lookup() for those.
lookup() method is called always. But method is called from
nfs_dir_inode_operations.
Correct sequence: nfs_follow_remote_path-> vfs_path_lookup->do_lookup->
->nfs_lookup->nfs_fhget->"reset i_op"->follow_link
Incorrect sequence: nfs4_get_root->nfs_fhget->"reset
i_op"->vfs_path_lookup->do_lookup
The problem is in that nfs_fhget resets i_op and then vfs_path_lookup
is called.
> That's the real issue...
Do you mean that we need to ignore updated attribute fsid for remote
ROOT at nfs4_get_root() ?
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-01-18 22:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-17 16:55 [nfs] Oops during call nfs_mountpoint_inode_operations->lookup() method Vitaliy Gusev
2011-01-17 17:04 ` Vitaliy Gusev
2011-01-17 22:18 ` Al Viro
2011-01-18 22:59 ` Vitaliy Gusev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).