Re: [GIT] More NFS client fixes for 2.6.24-rc6
On Thu, Jan 03, 2008 at 09:45:57AM -0500, Trond Myklebust wrote: >... > Thanks to Coverity/Adrian Bunk and Frank Filz for spotting the bug. >... I don't mind whether I'm credited (and the text can stay as it is), but I just want to note that I neither am nor was in any way affiliated with the Coverity people. > Cheers > Trond cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT] More NFS client fixes for 2.6.24-rc6
On Thu, 2008-01-03 at 10:30 +0100, Ingo Molnar wrote: > * Trond Myklebust <[EMAIL PROTECTED]> wrote: > > > commit 53478daff2c8b494d2af1ede6611f166f81bc393 > > Author: Trond Myklebust <[EMAIL PROTECTED]> > > Date: Wed Jan 2 13:28:57 2008 -0500 > > > > NFS: Fix a possible Oops in fs/nfs/super.c > > > > Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS > > mountpoint crossing...) had a slight flaw: server can be NULL if sget() > > returned an existing superblock. > > > > Fix the fix by dereferencing s->s_fs_info. > > > > Also add in the same namespace Oops fix for NFSv4 in both the mountpoint > > crossing case, and the referral case. > > > > Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]> > > shouldnt this commit have included the full credit of the bugfix: > > http://bugzilla.kernel.org/show_bug.cgi?id=9647 > > > > Description From Adrian Bunk 2007-12-27 12:36 > > The Coverity checker spotted that commit > 4584f520e1f773082ef44ff4f8969a5d992b16ec introduced the following NULL > dereference in 2.6.24-rc6: Point taken, however I assume that a reference to the bugzilla report should suffice. I've therefore updated the commit changelog to read as follows: NFS: Fix a possible Oops in fs/nfs/super.c Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS mountpoint crossing...) had a slight flaw: server can be NULL if sget() returned an existing superblock. Fix the fix by dereferencing s->s_fs_info. Thanks to Coverity/Adrian Bunk and Frank Filz for spotting the bug. (See http://bugzilla.kernel.org/show_bug.cgi?id=9647) Also add in the same namespace Oops fix for NFSv4 in both the mountpoint crossing case, and the referral case. Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]> Cheers Trond -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT] More NFS client fixes for 2.6.24-rc6
* Trond Myklebust <[EMAIL PROTECTED]> wrote:
> commit 53478daff2c8b494d2af1ede6611f166f81bc393
> Author: Trond Myklebust <[EMAIL PROTECTED]>
> Date: Wed Jan 2 13:28:57 2008 -0500
>
> NFS: Fix a possible Oops in fs/nfs/super.c
>
> Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS
> mountpoint crossing...) had a slight flaw: server can be NULL if sget()
> returned an existing superblock.
>
> Fix the fix by dereferencing s->s_fs_info.
>
> Also add in the same namespace Oops fix for NFSv4 in both the mountpoint
> crossing case, and the referral case.
>
> Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>
shouldnt this commit have included the full credit of the bugfix:
http://bugzilla.kernel.org/show_bug.cgi?id=9647
>
Description From Adrian Bunk 2007-12-27 12:36
The Coverity checker spotted that commit
4584f520e1f773082ef44ff4f8969a5d992b16ec introduced the following NULL
dereference in 2.6.24-rc6:
<-- snip -->
if (s->s_fs_info != server) {
nfs_free_server(server);
server = NULL; <---
}
--- Comment #1 From Adrian Bunk 2007-12-27 12:37:42 ---
The NULL dereference is at the
server->nfs_client->rpc_ops->dir_inode_ops.
<--
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT] More NFS client fixes for 2.6.24-rc6
* Trond Myklebust [EMAIL PROTECTED] wrote:
commit 53478daff2c8b494d2af1ede6611f166f81bc393
Author: Trond Myklebust [EMAIL PROTECTED]
Date: Wed Jan 2 13:28:57 2008 -0500
NFS: Fix a possible Oops in fs/nfs/super.c
Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS
mountpoint crossing...) had a slight flaw: server can be NULL if sget()
returned an existing superblock.
Fix the fix by dereferencing s-s_fs_info.
Also add in the same namespace Oops fix for NFSv4 in both the mountpoint
crossing case, and the referral case.
Signed-off-by: Trond Myklebust [EMAIL PROTECTED]
shouldnt this commit have included the full credit of the bugfix:
http://bugzilla.kernel.org/show_bug.cgi?id=9647
Description From Adrian Bunk 2007-12-27 12:36
The Coverity checker spotted that commit
4584f520e1f773082ef44ff4f8969a5d992b16ec introduced the following NULL
dereference in 2.6.24-rc6:
-- snip --
if (s-s_fs_info != server) {
nfs_free_server(server);
server = NULL; ---
}
--- Comment #1 From Adrian Bunk 2007-12-27 12:37:42 ---
The NULL dereference is at the
server-nfs_client-rpc_ops-dir_inode_ops.
--
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT] More NFS client fixes for 2.6.24-rc6
On Thu, 2008-01-03 at 10:30 +0100, Ingo Molnar wrote: * Trond Myklebust [EMAIL PROTECTED] wrote: commit 53478daff2c8b494d2af1ede6611f166f81bc393 Author: Trond Myklebust [EMAIL PROTECTED] Date: Wed Jan 2 13:28:57 2008 -0500 NFS: Fix a possible Oops in fs/nfs/super.c Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS mountpoint crossing...) had a slight flaw: server can be NULL if sget() returned an existing superblock. Fix the fix by dereferencing s-s_fs_info. Also add in the same namespace Oops fix for NFSv4 in both the mountpoint crossing case, and the referral case. Signed-off-by: Trond Myklebust [EMAIL PROTECTED] shouldnt this commit have included the full credit of the bugfix: http://bugzilla.kernel.org/show_bug.cgi?id=9647 Description From Adrian Bunk 2007-12-27 12:36 The Coverity checker spotted that commit 4584f520e1f773082ef44ff4f8969a5d992b16ec introduced the following NULL dereference in 2.6.24-rc6: Point taken, however I assume that a reference to the bugzilla report should suffice. I've therefore updated the commit changelog to read as follows: NFS: Fix a possible Oops in fs/nfs/super.c Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS mountpoint crossing...) had a slight flaw: server can be NULL if sget() returned an existing superblock. Fix the fix by dereferencing s-s_fs_info. Thanks to Coverity/Adrian Bunk and Frank Filz for spotting the bug. (See http://bugzilla.kernel.org/show_bug.cgi?id=9647) Also add in the same namespace Oops fix for NFSv4 in both the mountpoint crossing case, and the referral case. Signed-off-by: Trond Myklebust [EMAIL PROTECTED] Cheers Trond -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT] More NFS client fixes for 2.6.24-rc6
On Thu, Jan 03, 2008 at 09:45:57AM -0500, Trond Myklebust wrote: ... Thanks to Coverity/Adrian Bunk and Frank Filz for spotting the bug. ... I don't mind whether I'm credited (and the text can stay as it is), but I just want to note that I neither am nor was in any way affiliated with the Coverity people. Cheers Trond cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT] More NFS client fixes for 2.6.24-rc6
Hi Linus,
Please pull from the repository at
git pull git://git.linux-nfs.org/pub/linux/nfs-2.6.git
This will update the following files through the appended changesets.
Cheers,
Trond
fs/nfs/nfs4proc.c | 34 --
fs/nfs/nfs4renewd.c|2 --
fs/nfs/super.c | 12 +++-
net/sunrpc/auth_gss/auth_gss.c |2 +-
4 files changed, 32 insertions(+), 18 deletions(-)
commit 361562e89f02db8fdca32a2dfc546a74f5d883d9
Author: Trond Myklebust <[EMAIL PROTECTED]>
Date: Wed Jan 2 16:27:16 2008 -0500
NFSv4: Fix open_to_lock_owner sequenceid allocation...
NFSv4 file locking is currently completely broken since it doesn't respect
the OPEN sequencing when it is given an unconfirmed lock_owner and needs to
do an open_to_lock_owner. Worse: it breaks the sunrpc rules by doing a
GFP_KERNEL allocation inside an rpciod callback.
Fix is to preallocate the open seqid structure in nfs4_alloc_lockdata if we
see that the lock_owner is unconfirmed.
Then, in nfs4_lock_prepare() we wait for either the open_seqid, if
the lock_owner is still unconfirmed, or else fall back to waiting on the
standard lock_seqid.
Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>
commit 9073a07d1d987f8d2c919c900ea18ff57d156a34
Author: Trond Myklebust <[EMAIL PROTECTED]>
Date: Wed Jan 2 15:19:18 2008 -0500
NFSv4: nfs4_open_confirm must not set the open_owner as confirmed on error
RFC3530 states that the open_owner is confirmed if and only if the client
sends an OPEN_CONFIRM request with the appropriate sequence id and stateid
within the lease period.
Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>
commit 812f85c5715310583459acc4b35e918914e96060
Author: James Morris <[EMAIL PROTECTED]>
Date: Wed Dec 26 11:20:43 2007 +1100
NFS: add newline to kernel warning message in auth_gss code
Add newline to kernel warning message in gss_create().
Signed-off-by: James Morris <[EMAIL PROTECTED]>
Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>
commit 72d740f5d660bb422bab5f72da56678ca2c82d81
Author: Trond Myklebust <[EMAIL PROTECTED]>
Date: Wed Jan 2 13:52:03 2008 -0500
NFSv4: Fix circular locking dependency in nfs4_kill_renewd
Erez Zadok reports:
===
[ INFO: possible circular locking dependency detected ]
2.6.24-rc6-unionfs2 #80
---
umount.nfs4/4017 is trying to acquire lock:
(&(>cl_renewd)->work){--..}, at: []
__cancel_work_timer+0x83/0x17f
but task is already holding lock:
(>cl_sem){}, at: [] nfs4_kill_renewd+0x17/0x29 [nfs]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (>cl_sem){}:
[] __lock_acquire+0x9cc/0xb95
[] lock_acquire+0x5f/0x78
[] down_read+0x3a/0x4c
[] nfs4_renew_state+0x1c/0x1b8 [nfs]
[] run_workqueue+0xd9/0x1ac
[] worker_thread+0x7a/0x86
[] kthread+0x3b/0x62
[] kernel_thread_helper+0x7/0x10
[] 0x
-> #0 (&(>cl_renewd)->work){--..}:
[] __lock_acquire+0x8bc/0xb95
[] lock_acquire+0x5f/0x78
[] __cancel_work_timer+0xb7/0x17f
[] cancel_delayed_work_sync+0xb/0xd
[] nfs4_kill_renewd+0x1e/0x29 [nfs]
[] nfs_free_client+0x37/0x9e [nfs]
[] nfs_put_client+0x5d/0x62 [nfs]
[] nfs_free_server+0x75/0xae [nfs]
[] nfs4_kill_super+0x27/0x2b [nfs]
[] deactivate_super+0x3f/0x51
[] mntput_no_expire+0x42/0x67
[] path_release_on_umount+0x15/0x18
[] sys_umount+0x1a3/0x1cb
[] sys_oldumount+0x19/0x1b
[] sysenter_past_esp+0x5f/0xa5
[] 0x
Looking at the code, it would seem that taking the clp->cl_sem in
nfs4_kill_renewd is completely redundant, since we're already guaranteed to
have exclusive access to the nfs_client (we're shutting down).
Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>
commit 53478daff2c8b494d2af1ede6611f166f81bc393
Author: Trond Myklebust <[EMAIL PROTECTED]>
Date: Wed Jan 2 13:28:57 2008 -0500
NFS: Fix a possible Oops in fs/nfs/super.c
Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS
mountpoint crossing...) had a slight flaw: server can be NULL if sget()
returned an existing superblock.
Fix the fix by dereferencing s->s_fs_info.
Also add in the same namespace Oops fix for NFSv4 in both the mountpoint
crossing case, and the referral case.
Signed-off-by: Trond Myklebust <[EMAIL PROTECTED]>
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f03d9d5..9e2e1c7 100644
---
[GIT] More NFS client fixes for 2.6.24-rc6
Hi Linus,
Please pull from the repository at
git pull git://git.linux-nfs.org/pub/linux/nfs-2.6.git
This will update the following files through the appended changesets.
Cheers,
Trond
fs/nfs/nfs4proc.c | 34 --
fs/nfs/nfs4renewd.c|2 --
fs/nfs/super.c | 12 +++-
net/sunrpc/auth_gss/auth_gss.c |2 +-
4 files changed, 32 insertions(+), 18 deletions(-)
commit 361562e89f02db8fdca32a2dfc546a74f5d883d9
Author: Trond Myklebust [EMAIL PROTECTED]
Date: Wed Jan 2 16:27:16 2008 -0500
NFSv4: Fix open_to_lock_owner sequenceid allocation...
NFSv4 file locking is currently completely broken since it doesn't respect
the OPEN sequencing when it is given an unconfirmed lock_owner and needs to
do an open_to_lock_owner. Worse: it breaks the sunrpc rules by doing a
GFP_KERNEL allocation inside an rpciod callback.
Fix is to preallocate the open seqid structure in nfs4_alloc_lockdata if we
see that the lock_owner is unconfirmed.
Then, in nfs4_lock_prepare() we wait for either the open_seqid, if
the lock_owner is still unconfirmed, or else fall back to waiting on the
standard lock_seqid.
Signed-off-by: Trond Myklebust [EMAIL PROTECTED]
commit 9073a07d1d987f8d2c919c900ea18ff57d156a34
Author: Trond Myklebust [EMAIL PROTECTED]
Date: Wed Jan 2 15:19:18 2008 -0500
NFSv4: nfs4_open_confirm must not set the open_owner as confirmed on error
RFC3530 states that the open_owner is confirmed if and only if the client
sends an OPEN_CONFIRM request with the appropriate sequence id and stateid
within the lease period.
Signed-off-by: Trond Myklebust [EMAIL PROTECTED]
commit 812f85c5715310583459acc4b35e918914e96060
Author: James Morris [EMAIL PROTECTED]
Date: Wed Dec 26 11:20:43 2007 +1100
NFS: add newline to kernel warning message in auth_gss code
Add newline to kernel warning message in gss_create().
Signed-off-by: James Morris [EMAIL PROTECTED]
Signed-off-by: Trond Myklebust [EMAIL PROTECTED]
commit 72d740f5d660bb422bab5f72da56678ca2c82d81
Author: Trond Myklebust [EMAIL PROTECTED]
Date: Wed Jan 2 13:52:03 2008 -0500
NFSv4: Fix circular locking dependency in nfs4_kill_renewd
Erez Zadok reports:
===
[ INFO: possible circular locking dependency detected ]
2.6.24-rc6-unionfs2 #80
---
umount.nfs4/4017 is trying to acquire lock:
((clp-cl_renewd)-work){--..}, at: [c0223e53]
__cancel_work_timer+0x83/0x17f
but task is already holding lock:
(clp-cl_sem){}, at: [f8879897] nfs4_kill_renewd+0x17/0x29 [nfs]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
- #1 (clp-cl_sem){}:
[c0230699] __lock_acquire+0x9cc/0xb95
[c0230c39] lock_acquire+0x5f/0x78
[c0397cb8] down_read+0x3a/0x4c
[f88798e6] nfs4_renew_state+0x1c/0x1b8 [nfs]
[c0223821] run_workqueue+0xd9/0x1ac
[c0224220] worker_thread+0x7a/0x86
[c0226b49] kthread+0x3b/0x62
[c02033a3] kernel_thread_helper+0x7/0x10
[] 0x
- #0 ((clp-cl_renewd)-work){--..}:
[c0230589] __lock_acquire+0x8bc/0xb95
[c0230c39] lock_acquire+0x5f/0x78
[c0223e87] __cancel_work_timer+0xb7/0x17f
[c0223f5a] cancel_delayed_work_sync+0xb/0xd
[f887989e] nfs4_kill_renewd+0x1e/0x29 [nfs]
[f885a8f6] nfs_free_client+0x37/0x9e [nfs]
[f885ab20] nfs_put_client+0x5d/0x62 [nfs]
[f885ab9a] nfs_free_server+0x75/0xae [nfs]
[f8862672] nfs4_kill_super+0x27/0x2b [nfs]
[c0258aab] deactivate_super+0x3f/0x51
[c0269668] mntput_no_expire+0x42/0x67
[c025d0e4] path_release_on_umount+0x15/0x18
[c0269d30] sys_umount+0x1a3/0x1cb
[c0269d71] sys_oldumount+0x19/0x1b
[c02026ca] sysenter_past_esp+0x5f/0xa5
[] 0x
Looking at the code, it would seem that taking the clp-cl_sem in
nfs4_kill_renewd is completely redundant, since we're already guaranteed to
have exclusive access to the nfs_client (we're shutting down).
Signed-off-by: Trond Myklebust [EMAIL PROTECTED]
commit 53478daff2c8b494d2af1ede6611f166f81bc393
Author: Trond Myklebust [EMAIL PROTECTED]
Date: Wed Jan 2 13:28:57 2008 -0500
NFS: Fix a possible Oops in fs/nfs/super.c
Sigh... commit 4584f520e1f773082ef44ff4f8969a5d992b16ec (NFS: Fix NFS
mountpoint crossing...) had a slight flaw: server can be NULL if sget()
returned an existing superblock.
Fix the fix by dereferencing s-s_fs_info.
Also add in the same namespace Oops fix for NFSv4 in both the
