* [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
@ 2011-06-09 12:29 Pavel Shilovsky
[not found] ` <1307622570-7141-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Shilovsky @ 2011-06-09 12:29 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Recently merged shared-sb capability causes kernel to crash when
we try to do mounts and umounts simultaneously on the same machine.
The patch fixes umount codepath by doing kill_anon_super at first
(it calls generic_shutdown_super that tries to take sb_lock and
removes a superblock from fs_supers list) and then processing
cifs_umount (that frees cifs related fields).
In this case when we call sget and it calls cifs_match_super under
sb_lock for every entry from fs_supers list, we can be sure that
all cifs related fields are not freed by simultaneous umount.
Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
fs/cifs/cifsfs.c | 19 ++++++++++++++++---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 989442d..3537e7d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -180,16 +180,30 @@ out_mount_failed:
static void
cifs_put_super(struct super_block *sb)
{
+ struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+ if (cifs_sb == NULL) {
+ cFYI(1, "Empty cifs superblock info passed to put_super");
+ return;
+ }
+
+ bdi_destroy(&cifs_sb->bdi);
+}
+
+static void
+cifs_kill_super(struct super_block *sb)
+{
int rc = 0;
struct cifs_sb_info *cifs_sb;
cFYI(1, "In cifs_put_super");
cifs_sb = CIFS_SB(sb);
if (cifs_sb == NULL) {
- cFYI(1, "Empty cifs superblock info passed to unmount");
+ cFYI(1, "Empty cifs superblock info passed to kill_super");
return;
}
+ kill_anon_super(sb);
+
rc = cifs_umount(sb, cifs_sb);
if (rc)
cERROR(1, "cifs_umount failed with return code %d", rc);
@@ -199,7 +213,6 @@ cifs_put_super(struct super_block *sb)
}
unload_nls(cifs_sb->local_nls);
- bdi_destroy(&cifs_sb->bdi);
kfree(cifs_sb);
}
@@ -807,7 +820,7 @@ struct file_system_type cifs_fs_type = {
.owner = THIS_MODULE,
.name = "cifs",
.mount = cifs_do_mount,
- .kill_sb = kill_anon_super,
+ .kill_sb = cifs_kill_super,
/* .fs_flags */
};
const struct inode_operations cifs_dir_inode_ops = {
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <1307622570-7141-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2011-06-09 12:40 ` Christoph Hellwig
[not found] ` <20110609124043.GA9621-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2011-06-09 12:40 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
> static void
> cifs_put_super(struct super_block *sb)
> {
> + struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
> + if (cifs_sb == NULL) {
> + cFYI(1, "Empty cifs superblock info passed to put_super");
> + return;
> + }
> +
> + bdi_destroy(&cifs_sb->bdi);
This means you have a problem with the lifetime rules in cifs_do_mount.
The VFS only calls ->put_super if sb->s_root is set. So the rules for
the mount handler are to only set s_root once the superblock is fully
set up.
Also you should never call cifs_umount from the error handling in
cifs_do_mount. Until s_root is set, please unwind manually, after
that leave it to ->put_super.
> +
> +static void
> +cifs_kill_super(struct super_block *sb)
> +{
This also seems quite broken. If you fix up the mount path like
I suggested it won't be nessecary.
> int rc = 0;
> struct cifs_sb_info *cifs_sb;
>
> cFYI(1, "In cifs_put_super");
> cifs_sb = CIFS_SB(sb);
> if (cifs_sb == NULL) {
And this check should also be removed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <20110609124043.GA9621-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2011-06-09 16:30 ` Pavel Shilovsky
[not found] ` <BANLkTikmfKtDxG_6qg+6JiCT2FzF1FFfQg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Shilovsky @ 2011-06-09 16:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
2011/6/9 Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>:
>> static void
>> cifs_put_super(struct super_block *sb)
>> {
>> + struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>> + if (cifs_sb == NULL) {
>> + cFYI(1, "Empty cifs superblock info passed to put_super");
>> + return;
>> + }
>> +
>> + bdi_destroy(&cifs_sb->bdi);
>
> This means you have a problem with the lifetime rules in cifs_do_mount.
>
> The VFS only calls ->put_super if sb->s_root is set. So the rules for
> the mount handler are to only set s_root once the superblock is fully
> set up.
>
> Also you should never call cifs_umount from the error handling in
> cifs_do_mount. Until s_root is set, please unwind manually, after
> that leave it to ->put_super.
>
>
>> +
>> +static void
>> +cifs_kill_super(struct super_block *sb)
>> +{
>
> This also seems quite broken. If you fix up the mount path like
> I suggested it won't be nessecary.
>
>> int rc = 0;
>> struct cifs_sb_info *cifs_sb;
>>
>> cFYI(1, "In cifs_put_super");
>> cifs_sb = CIFS_SB(sb);
>> if (cifs_sb == NULL) {
>
> And this check should also be removed.
>
>
It seems to me that I didn't understand your points clearly.
That's how I reproduce the problem:
1) one process does mount/umount calls in a loop;
2) another process does mount and umount calls.
As the result, kernel crashes sometimes on mount of the second
process, sometimes - on umount of the second process. The problem is
that:
one of these two processes process umount call. So, it comes to
cifs_put_super, calls cifs_umount and then calls kfree(cifs_sb). In
the same time, another process comes into cifs_do_mount and calls
sget(). Then it appers into cifs_match_super that thinks that all
structures like server, ses, tcon and cifs_sb are valid, but it is not
true - the first process has already freed them but didn't remove
superblock from fs_supers list.
So, I simply reodered calls: now umount codepath marks a superblock as
non-active and calls kill_sb(new cifs_kill_super), it calls
kill_anon_super (that calls cifs_put_super and removes superblock from
fs_supers list) and after that kill_sb frees all cifs structures
(server, ses, tcon, cifs_sb).
As the result, when we get into cifs_match_super (in mount from
another process) we are sure that all cifs structures are valid.
If I am not right, correct me, please!
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <BANLkTikmfKtDxG_6qg+6JiCT2FzF1FFfQg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-06-10 6:00 ` Pavel Shilovsky
2011-06-11 9:43 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Pavel Shilovsky @ 2011-06-10 6:00 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
2011/6/9 Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> 2011/6/9 Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>:
>>> static void
>>> cifs_put_super(struct super_block *sb)
>>> {
>>> + struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>>> + if (cifs_sb == NULL) {
>>> + cFYI(1, "Empty cifs superblock info passed to put_super");
>>> + return;
>>> + }
>>> +
>>> + bdi_destroy(&cifs_sb->bdi);
>>
>> This means you have a problem with the lifetime rules in cifs_do_mount.
>>
>> The VFS only calls ->put_super if sb->s_root is set. So the rules for
>> the mount handler are to only set s_root once the superblock is fully
>> set up.
>>
>> Also you should never call cifs_umount from the error handling in
>> cifs_do_mount. Until s_root is set, please unwind manually, after
>> that leave it to ->put_super.
>>
>>
>>> +
>>> +static void
>>> +cifs_kill_super(struct super_block *sb)
>>> +{
>>
>> This also seems quite broken. If you fix up the mount path like
>> I suggested it won't be nessecary.
>>
>>> int rc = 0;
>>> struct cifs_sb_info *cifs_sb;
>>>
>>> cFYI(1, "In cifs_put_super");
>>> cifs_sb = CIFS_SB(sb);
>>> if (cifs_sb == NULL) {
>>
>> And this check should also be removed.
>>
>>
>
> It seems to me that I didn't understand your points clearly.
>
> That's how I reproduce the problem:
> 1) one process does mount/umount calls in a loop;
> 2) another process does mount and umount calls.
>
> As the result, kernel crashes sometimes on mount of the second
> process, sometimes - on umount of the second process. The problem is
> that:
>
> one of these two processes process umount call. So, it comes to
> cifs_put_super, calls cifs_umount and then calls kfree(cifs_sb). In
> the same time, another process comes into cifs_do_mount and calls
> sget(). Then it appers into cifs_match_super that thinks that all
> structures like server, ses, tcon and cifs_sb are valid, but it is not
> true - the first process has already freed them but didn't remove
> superblock from fs_supers list.
>
> So, I simply reodered calls: now umount codepath marks a superblock as
> non-active and calls kill_sb(new cifs_kill_super), it calls
> kill_anon_super (that calls cifs_put_super and removes superblock from
> fs_supers list) and after that kill_sb frees all cifs structures
> (server, ses, tcon, cifs_sb).
>
> As the result, when we get into cifs_match_super (in mount from
> another process) we are sure that all cifs structures are valid.
>
> If I am not right, correct me, please!
>
> --
> Best regards,
> Pavel Shilovsky.
>
I also want to mention that NFS mount code uses the same order:
1) in moun at first it sets up server structure and then it calls sget.
2) it has put_super that only calls bdi_unregister and kill_super that
at first call kill_anon_super and then frees server structure.
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <BANLkTikmfKtDxG_6qg+6JiCT2FzF1FFfQg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-06-10 6:00 ` Pavel Shilovsky
@ 2011-06-11 9:43 ` Christoph Hellwig
[not found] ` <20110611094314.GA6217-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
1 sibling, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2011-06-11 9:43 UTC (permalink / raw)
To: Pavel Shilovsky
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
> one of these two processes process umount call. So, it comes to
> cifs_put_super, calls cifs_umount and then calls kfree(cifs_sb). In
> the same time, another process comes into cifs_do_mount and calls
> sget(). Then it appers into cifs_match_super that thinks that all
> structures like server, ses, tcon and cifs_sb are valid, but it is not
> true - the first process has already freed them but didn't remove
> superblock from fs_supers list.
>
> So, I simply reodered calls: now umount codepath marks a superblock as
> non-active and calls kill_sb(new cifs_kill_super), it calls
> kill_anon_super (that calls cifs_put_super and removes superblock from
> fs_supers list) and after that kill_sb frees all cifs structures
> (server, ses, tcon, cifs_sb).
>
> As the result, when we get into cifs_match_super (in mount from
> another process) we are sure that all cifs structures are valid.
>
> If I am not right, correct me, please!
Ok, if we look at sb-private data in the sget callsbacks it seems like
the clreanup for those does indeed need to be done in ->kill_sb. I have
to say I really hate it, and the culprit is that we call the sget test
callback is called before we call grab_super in sget, that is we don't
protect against filesystems going away. I suspect that is the real
problem here.
The workaround you've copied from NFS also works, but it's pretty
ugly, given that ->kill_sb also is called for partially set up
superblocks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <20110611094314.GA6217-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2011-06-11 11:12 ` Al Viro
[not found] ` <20110611111253.GD11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Al Viro @ 2011-06-11 11:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Pavel Shilovsky, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Sat, Jun 11, 2011 at 05:43:14AM -0400, Christoph Hellwig wrote:
> Ok, if we look at sb-private data in the sget callsbacks it seems like
> the clreanup for those does indeed need to be done in ->kill_sb. I have
> to say I really hate it, and the culprit is that we call the sget test
> callback is called before we call grab_super in sget, that is we don't
> protect against filesystems going away. I suspect that is the real
> problem here.
grab_super() is *heavy* and having to undo it even once means that we need to
rescan. Sorry, test() has to be callable without that.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <20110611111253.GD11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
@ 2011-06-11 14:00 ` Pavel Shilovsky
[not found] ` <BANLkTim==J7=-bEz1pahhKuxFdMFe2BnWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Shilovsky @ 2011-06-11 14:00 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Al Viro, linux-cifs-u79uwXL29TY76Z2rM5mHXA
2011/6/11 Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>:
> On Sat, Jun 11, 2011 at 05:43:14AM -0400, Christoph Hellwig wrote:
>
>> Ok, if we look at sb-private data in the sget callsbacks it seems like
>> the clreanup for those does indeed need to be done in ->kill_sb. I have
>> to say I really hate it, and the culprit is that we call the sget test
>> callback is called before we call grab_super in sget, that is we don't
>> protect against filesystems going away. I suspect that is the real
>> problem here.
>
> grab_super() is *heavy* and having to undo it even once means that we need to
> rescan. Sorry, test() has to be callable without that.
>
In this case, I think we should follow NFS strategy and apply my patch
as a workaround. Thoughts?
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <BANLkTim==J7=-bEz1pahhKuxFdMFe2BnWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-06-17 3:54 ` Al Viro
[not found] ` <20110617035421.GO11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Al Viro @ 2011-06-17 3:54 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: Christoph Hellwig, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Sat, Jun 11, 2011 at 06:00:25PM +0400, Pavel Shilovsky wrote:
> 2011/6/11 Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>:
> > On Sat, Jun 11, 2011 at 05:43:14AM -0400, Christoph Hellwig wrote:
> >
> >> Ok, if we look at sb-private data in the sget callsbacks it seems like
> >> the clreanup for those does indeed need to be done in ->kill_sb. ?I have
> >> to say I really hate it, and the culprit is that we call the sget test
> >> callback is called before we call grab_super in sget, that is we don't
> >> protect against filesystems going away. ?I suspect that is the real
> >> problem here.
> >
> > grab_super() is *heavy* and having to undo it even once means that we need to
> > rescan. ?Sorry, test() has to be callable without that.
> >
>
> In this case, I think we should follow NFS strategy and apply my patch
> as a workaround. Thoughts?
See #untested in vfs-2.6.git; I *hope* I've got all the leaks in current
mainline plugged and all sget() races ought to be taken care of. And
yes, the branch name matches the reality - this sucker is completely
untested. Have fun... Commit message is atrocious, of course - I'm too
tired to even try writing a coherent patch description at the moment and
I'll probably need to split it into several patches anyway. Tomorrow...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <20110617035421.GO11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
@ 2011-06-17 14:08 ` Al Viro
[not found] ` <20110617140833.GA9563-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Al Viro @ 2011-06-17 14:08 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: Christoph Hellwig, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Fri, Jun 17, 2011 at 04:54:21AM +0100, Al Viro wrote:
> See #untested in vfs-2.6.git; I *hope* I've got all the leaks in current
> mainline plugged and all sget() races ought to be taken care of. And
> yes, the branch name matches the reality - this sucker is completely
> untested. Have fun... Commit message is atrocious, of course - I'm too
> tired to even try writing a coherent patch description at the moment and
> I'll probably need to split it into several patches anyway. Tomorrow...
Updated, split up, pushed into #untested. Enjoy...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <20110617140833.GA9563-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
@ 2011-06-20 12:54 ` Pavel Shilovsky
[not found] ` <BANLkTi=mH-9F9N4Gz8JJqL==Hm5p8Y1_tQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Shilovsky @ 2011-06-20 12:54 UTC (permalink / raw)
To: Al Viro, Steve French, Jeff Layton
Cc: Christoph Hellwig, linux-cifs-u79uwXL29TY76Z2rM5mHXA
2011/6/17 Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>:
> On Fri, Jun 17, 2011 at 04:54:21AM +0100, Al Viro wrote:
>
>> See #untested in vfs-2.6.git; I *hope* I've got all the leaks in current
>> mainline plugged and all sget() races ought to be taken care of. And
>> yes, the branch name matches the reality - this sucker is completely
>> untested. Have fun... Commit message is atrocious, of course - I'm too
>> tired to even try writing a coherent patch description at the moment and
>> I'll probably need to split it into several patches anyway. Tomorrow...
>
> Updated, split up, pushed into #untested. Enjoy...
>
Today I tried this branch and it was ok.
Steve, Jeff, what do you think?
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <BANLkTi=mH-9F9N4Gz8JJqL==Hm5p8Y1_tQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-06-20 12:59 ` Jeff Layton
[not found] ` <20110620085943.6371af1b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-06-20 12:59 UTC (permalink / raw)
To: Pavel Shilovsky
Cc: Al Viro, Steve French, Christoph Hellwig,
linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Mon, 20 Jun 2011 16:54:48 +0400
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2011/6/17 Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>:
> > On Fri, Jun 17, 2011 at 04:54:21AM +0100, Al Viro wrote:
> >
> >> See #untested in vfs-2.6.git; I *hope* I've got all the leaks in current
> >> mainline plugged and all sget() races ought to be taken care of. And
> >> yes, the branch name matches the reality - this sucker is completely
> >> untested. Have fun... Commit message is atrocious, of course - I'm too
> >> tired to even try writing a coherent patch description at the moment and
> >> I'll probably need to split it into several patches anyway. Tomorrow...
> >
> > Updated, split up, pushed into #untested. Enjoy...
> >
>
> Today I tried this branch and it was ok.
>
> Steve, Jeff, what do you think?
>
I did some light testing on it over the weekend and it looks good to me
as well. Nice cleanup, Al.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <20110620085943.6371af1b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2011-06-20 16:13 ` Steve French
[not found] ` <BANLkTinfETnDdcBFkQNq8pLZPQOGsVRuGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Steve French @ 2011-06-20 16:13 UTC (permalink / raw)
To: Jeff Layton
Cc: Pavel Shilovsky, Al Viro, Christoph Hellwig,
linux-cifs-u79uwXL29TY76Z2rM5mHXA
Would the plan be for Al to merge these out of his tree?
On Mon, Jun 20, 2011 at 7:59 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 20 Jun 2011 16:54:48 +0400
> Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> 2011/6/17 Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>:
>> > On Fri, Jun 17, 2011 at 04:54:21AM +0100, Al Viro wrote:
>> >
>> >> See #untested in vfs-2.6.git; I *hope* I've got all the leaks in current
>> >> mainline plugged and all sget() races ought to be taken care of. And
>> >> yes, the branch name matches the reality - this sucker is completely
>> >> untested. Have fun... Commit message is atrocious, of course - I'm too
>> >> tired to even try writing a coherent patch description at the moment and
>> >> I'll probably need to split it into several patches anyway. Tomorrow...
>> >
>> > Updated, split up, pushed into #untested. Enjoy...
>> >
>>
>> Today I tried this branch and it was ok.
>>
>> Steve, Jeff, what do you think?
>>
>
> I did some light testing on it over the weekend and it looks good to me
> as well. Nice cleanup, Al.
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <BANLkTinfETnDdcBFkQNq8pLZPQOGsVRuGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-06-20 16:15 ` Al Viro
[not found] ` <20110620161516.GU11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Al Viro @ 2011-06-20 16:15 UTC (permalink / raw)
To: Steve French
Cc: Jeff Layton, Pavel Shilovsky, Christoph Hellwig,
linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Mon, Jun 20, 2011 at 11:13:22AM -0500, Steve French wrote:
> Would the plan be for Al to merge these out of his tree?
Can do, if cifs folks are OK with it... Note that it's a regression since
the last merge window, so it probably ought to go to Linus before 3.0-final.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <20110620161516.GU11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
@ 2011-06-24 8:47 ` Pavel Shilovsky
[not found] ` <BANLkTi=nYfiy0qnHPXs2fckCPNJXwbpi7Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Shilovsky @ 2011-06-24 8:47 UTC (permalink / raw)
To: Al Viro
Cc: Steve French, Jeff Layton, Christoph Hellwig,
linux-cifs-u79uwXL29TY76Z2rM5mHXA
2011/6/20 Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>:
> On Mon, Jun 20, 2011 at 11:13:22AM -0500, Steve French wrote:
>> Would the plan be for Al to merge these out of his tree?
>
> Can do, if cifs folks are OK with it... Note that it's a regression since
> the last merge window, so it probably ought to go to Linus before 3.0-final.
>
Al, what is the status of your patchset? Note, that you can add my
"Acked-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>" tag as well if you
need it.
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls
[not found] ` <BANLkTi=nYfiy0qnHPXs2fckCPNJXwbpi7Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-06-24 10:48 ` Jeff Layton
[not found] ` <20110624064810.495dec99-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-06-24 10:48 UTC (permalink / raw)
To: Pavel Shilovsky
Cc: Al Viro, Steve French, Christoph Hellwig,
linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Fri, 24 Jun 2011 12:47:42 +0400
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2011/6/20 Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>:
> > On Mon, Jun 20, 2011 at 11:13:22AM -0500, Steve French wrote:
> >> Would the plan be for Al to merge these out of his tree?
> >
> > Can do, if cifs folks are OK with it... Note that it's a regression since
> > the last merge window, so it probably ought to go to Linus before 3.0-final.
> >
>
> Al, what is the status of your patchset? Note, that you can add my
> "Acked-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>" tag as well if you
> need it.
>
Yes, I've looked over the set and it looks good to me. Nice cleanup,
Al. You can add my:
Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [git pull] ->mount() regression fixes for cifs
[not found] ` <20110624064810.495dec99-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2011-06-24 22:54 ` Al Viro
[not found] ` <20110624225414.GA11013-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Al Viro @ 2011-06-24 22:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: Pavel Shilovsky, Steve French, Christoph Hellwig,
linux-cifs-u79uwXL29TY76Z2rM5mHXA, Jeff Layton
> > Al, what is the status of your patchset? Note, that you can add my
> > "Acked-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>" tag as well if you
> > need it.
> >
>
> Yes, I've looked over the set and it looks good to me. Nice cleanup,
> Al. You can add my:
>
> Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
OK... The patchset fixes breakage that got into cifs ->mount() since
cifs had started to play with shared superblocks - sget() races, leaks,
etc. Commit dates are recent due to added Acked-by and Reviewed-by;
other than that, it's an exact copy of the stuff that sat in for-next.
What is the proper way to deal with such situations, BTW? I know that
you seriously dislike being asked to pull just-created commits, but
the normal reasons do not apply in this case... Please, pull from
the usual place -
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ for-linus
Shortlog:
Al Viro (15):
take bdi setup/destruction into cifs_mount/cifs_umount
cifs: double free on mount failure
cifs: don't leak nls on mount failure
cifs: don't pass superblock to cifs_mount()
cifs: leak on mount if we share superblock
cifs: allocate mountdata earlier
cifs: initialize ->tlink_tree in cifs_setup_cifs_sb()
sanitize cifs_umount() prototype
cifs: pull cifs_mount() call up
cifs: move cifs_umount() call into ->kill_sb()
cifs: pull freeing mountdata/dropping nls/freeing cifs_sb into cifs_umount()
cifs: close sget() races
cifs: more breakage on mount failures
cifs: tidy cifs_do_mount() up a bit
cifs: propagate errors from cifs_get_root() to mount(2)
Diffstat:
fs/cifs/cifs_fs_sb.h | 1 +
fs/cifs/cifsfs.c | 156 +++++++++++++++++++++-----------------------------
fs/cifs/cifsproto.h | 8 +-
fs/cifs/connect.c | 49 +++++++++-------
4 files changed, 98 insertions(+), 116 deletions(-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] ->mount() regression fixes for cifs
[not found] ` <20110624225414.GA11013-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
@ 2011-06-24 23:03 ` Steve French
0 siblings, 0 replies; 17+ messages in thread
From: Steve French @ 2011-06-24 23:03 UTC (permalink / raw)
To: Al Viro
Cc: Linus Torvalds, Pavel Shilovsky, Christoph Hellwig,
linux-cifs-u79uwXL29TY76Z2rM5mHXA, Jeff Layton
I have reviewed most of it and am fine with it going out of your tree.
On Fri, Jun 24, 2011 at 5:54 PM, Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org> wrote:
>> > Al, what is the status of your patchset? Note, that you can add my
>> > "Acked-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>" tag as well if you
>> > need it.
>> >
>>
>> Yes, I've looked over the set and it looks good to me. Nice cleanup,
>> Al. You can add my:
>>
>> Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> OK... The patchset fixes breakage that got into cifs ->mount() since
> cifs had started to play with shared superblocks - sget() races, leaks,
> etc. Commit dates are recent due to added Acked-by and Reviewed-by;
> other than that, it's an exact copy of the stuff that sat in for-next.
> What is the proper way to deal with such situations, BTW? I know that
> you seriously dislike being asked to pull just-created commits, but
> the normal reasons do not apply in this case... Please, pull from
> the usual place -
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ for-linus
>
> Shortlog:
> Al Viro (15):
> take bdi setup/destruction into cifs_mount/cifs_umount
> cifs: double free on mount failure
> cifs: don't leak nls on mount failure
> cifs: don't pass superblock to cifs_mount()
> cifs: leak on mount if we share superblock
> cifs: allocate mountdata earlier
> cifs: initialize ->tlink_tree in cifs_setup_cifs_sb()
> sanitize cifs_umount() prototype
> cifs: pull cifs_mount() call up
> cifs: move cifs_umount() call into ->kill_sb()
> cifs: pull freeing mountdata/dropping nls/freeing cifs_sb into cifs_umount()
> cifs: close sget() races
> cifs: more breakage on mount failures
> cifs: tidy cifs_do_mount() up a bit
> cifs: propagate errors from cifs_get_root() to mount(2)
>
> Diffstat:
> fs/cifs/cifs_fs_sb.h | 1 +
> fs/cifs/cifsfs.c | 156 +++++++++++++++++++++-----------------------------
> fs/cifs/cifsproto.h | 8 +-
> fs/cifs/connect.c | 49 +++++++++-------
> 4 files changed, 98 insertions(+), 116 deletions(-)
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-06-24 23:03 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-09 12:29 [PATCH] CIFS: Fix kernel crash on simultaneous mount/umount calls Pavel Shilovsky
[not found] ` <1307622570-7141-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2011-06-09 12:40 ` Christoph Hellwig
[not found] ` <20110609124043.GA9621-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2011-06-09 16:30 ` Pavel Shilovsky
[not found] ` <BANLkTikmfKtDxG_6qg+6JiCT2FzF1FFfQg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-06-10 6:00 ` Pavel Shilovsky
2011-06-11 9:43 ` Christoph Hellwig
[not found] ` <20110611094314.GA6217-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2011-06-11 11:12 ` Al Viro
[not found] ` <20110611111253.GD11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2011-06-11 14:00 ` Pavel Shilovsky
[not found] ` <BANLkTim==J7=-bEz1pahhKuxFdMFe2BnWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-06-17 3:54 ` Al Viro
[not found] ` <20110617035421.GO11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2011-06-17 14:08 ` Al Viro
[not found] ` <20110617140833.GA9563-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2011-06-20 12:54 ` Pavel Shilovsky
[not found] ` <BANLkTi=mH-9F9N4Gz8JJqL==Hm5p8Y1_tQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-06-20 12:59 ` Jeff Layton
[not found] ` <20110620085943.6371af1b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-06-20 16:13 ` Steve French
[not found] ` <BANLkTinfETnDdcBFkQNq8pLZPQOGsVRuGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-06-20 16:15 ` Al Viro
[not found] ` <20110620161516.GU11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2011-06-24 8:47 ` Pavel Shilovsky
[not found] ` <BANLkTi=nYfiy0qnHPXs2fckCPNJXwbpi7Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-06-24 10:48 ` Jeff Layton
[not found] ` <20110624064810.495dec99-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-06-24 22:54 ` [git pull] ->mount() regression fixes for cifs Al Viro
[not found] ` <20110624225414.GA11013-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2011-06-24 23:03 ` Steve French
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.