* [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).