On Tue, Oct 23, 2018 at 12:19:35PM +0100, Alan Jenkins wrote:
> I think there's another small hole. It is possible to move a sub-mount from
> a detached tree (instead of moving the root of the tree). Then
> do_move_mount() calls attach_recursive_mnt() with a non-NULL parent_path.
>
> This
On Tue, Oct 23, 2018 at 12:19:35PM +0100, Alan Jenkins wrote:
> I think there's another small hole. It is possible to move a sub-mount from
> a detached tree (instead of moving the root of the tree). Then
> do_move_mount() calls attach_recursive_mnt() with a non-NULL parent_path.
>
> This
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
not detached anymore, it won't be dissolved. move_mount(2) is
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
not detached anymore, it won't be dissolved. move_mount(2) is
David Howells writes:
> From: Al Viro
>
> Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
> attached by move_mount(2).
>
> If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
> not detached anymore, it won't be dissolved. move_mount(2) is adjusted
>
David Howells writes:
> From: Al Viro
>
> Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
> attached by move_mount(2).
>
> If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
> not detached anymore, it won't be dissolved. move_mount(2) is adjusted
>
Alan Jenkins wrote:
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 4dfe7e23b7ee..e8d61d5f581d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1763,7 +1763,7 @@ void dissolve_on_fput(struct vfsmount *mnt)
> {
> namespace_lock();
> lock_mount_hash();
> - if
Alan Jenkins wrote:
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 4dfe7e23b7ee..e8d61d5f581d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1763,7 +1763,7 @@ void dissolve_on_fput(struct vfsmount *mnt)
> {
> namespace_lock();
> lock_mount_hash();
> - if
On Sat, Oct 20, 2018 at 12:48:26PM +0100, Al Viro wrote:
> Not just refcounting; it's that fs_pin is really intended to have ->kill()
> triggered only once. If you look at the pin_kill() (which is where the
> livelock happened)
More specifically, it's group_pin_kill() assuming that by the time
On Sat, Oct 20, 2018 at 12:48:26PM +0100, Al Viro wrote:
> Not just refcounting; it's that fs_pin is really intended to have ->kill()
> triggered only once. If you look at the pin_kill() (which is where the
> livelock happened)
More specifically, it's group_pin_kill() assuming that by the time
On Sat, Oct 20, 2018 at 12:06:32PM +0100, Alan Jenkins wrote:
> You posted an analysis of a GPF, where you showed the reference count was
> clearly one less than it should have been. You narrowed this down to a step
> where you connected an unmounted mount (MNT_UMOUNT) to a mounted mount. So
>
On Sat, Oct 20, 2018 at 12:06:32PM +0100, Alan Jenkins wrote:
> You posted an analysis of a GPF, where you showed the reference count was
> clearly one less than it should have been. You narrowed this down to a step
> where you connected an unmounted mount (MNT_UMOUNT) to a mounted mount. So
>
On 19/10/2018 23:36, David Howells wrote:
Alan Jenkins wrote:
# open_tree_clone 3
The reason EBUSY is returned is because propagate_mount_busy() is called by
do_umount() with refcnt == 2, but mnt_count == 3:
umount-3577 M=f8898a34 u=3 0x555 sp=__x64_sys_umount+0x12/0x15
the
On 19/10/2018 23:36, David Howells wrote:
Alan Jenkins wrote:
# open_tree_clone 3
The reason EBUSY is returned is because propagate_mount_busy() is called by
do_umount() with refcnt == 2, but mnt_count == 3:
umount-3577 M=f8898a34 u=3 0x555 sp=__x64_sys_umount+0x12/0x15
the
On Fri, Oct 19, 2018 at 11:36:19PM +0100, David Howells wrote:
> Alan Jenkins wrote:
>
> > # open_tree_clone 3 > # cd /proc/self/fd/3
> > # mount --move . /mnt
> > [ 41.747831] mnt_flags=1020 umount=0
> > # cd /
> > # umount /mnt
> > umount: /mnt: target is busy
> >
> > ^ a newly introduced
On Fri, Oct 19, 2018 at 11:36:19PM +0100, David Howells wrote:
> Alan Jenkins wrote:
>
> > # open_tree_clone 3 > # cd /proc/self/fd/3
> > # mount --move . /mnt
> > [ 41.747831] mnt_flags=1020 umount=0
> > # cd /
> > # umount /mnt
> > umount: /mnt: target is busy
> >
> > ^ a newly introduced
Alan Jenkins wrote:
> # open_tree_clone 3 # cd /proc/self/fd/3
> # mount --move . /mnt
> [ 41.747831] mnt_flags=1020 umount=0
> # cd /
> # umount /mnt
> umount: /mnt: target is busy
>
> ^ a newly introduced bug? I do not remember having this problem before.
The reason EBUSY is returned is
Alan Jenkins wrote:
> # open_tree_clone 3 # cd /proc/self/fd/3
> # mount --move . /mnt
> [ 41.747831] mnt_flags=1020 umount=0
> # cd /
> # umount /mnt
> umount: /mnt: target is busy
>
> ^ a newly introduced bug? I do not remember having this problem before.
The reason EBUSY is returned is
Alan Jenkins wrote:
> I guess this tries to fix the second of the two sequences I mentioned -
> mount+unmount, then close the FD. It doesn't seem to work.
It fixes this:
unshare --mount
/root/test-fsmount
mount --move . /mnt
mount --move /mnt /mnt
cd
umount /mnt
Alan Jenkins wrote:
> I guess this tries to fix the second of the two sequences I mentioned -
> mount+unmount, then close the FD. It doesn't seem to work.
It fixes this:
unshare --mount
/root/test-fsmount
mount --move . /mnt
mount --move /mnt /mnt
cd
umount /mnt
Alan Jenkins wrote:
> And the first sequence I mentioned - close the FD, then mount+unmount -
> seems to be unchanged.
Unchanged in what sense? Still breaks? I thought I'd fixed that - or are we
talking about a different first sequence?
Sorry, I'm losing track of how many different ways of
Alan Jenkins wrote:
> And the first sequence I mentioned - close the FD, then mount+unmount -
> seems to be unchanged.
Unchanged in what sense? Still breaks? I thought I'd fixed that - or are we
talking about a different first sequence?
Sorry, I'm losing track of how many different ways of
On 19/10/2018 14:37, David Howells wrote:
Alan Jenkins wrote:
If I close() the mount FD "mfd", and then do "mount --move . /mnt", my
printk() shows MNT_UMOUNT has been set. ( I guess fchdir() works more like
openat(... , O_PATH) than dup() ). Then unmounting /mnt hangs, as I would
expect from
On 19/10/2018 14:37, David Howells wrote:
Alan Jenkins wrote:
If I close() the mount FD "mfd", and then do "mount --move . /mnt", my
printk() shows MNT_UMOUNT has been set. ( I guess fchdir() works more like
openat(... , O_PATH) than dup() ). Then unmounting /mnt hangs, as I would
expect from
Alan Jenkins wrote:
> If I close() the mount FD "mfd", and then do "mount --move . /mnt", my
> printk() shows MNT_UMOUNT has been set. ( I guess fchdir() works more like
> openat(... , O_PATH) than dup() ). Then unmounting /mnt hangs, as I would
> expect from my previous test.
Okay, I think the
Alan Jenkins wrote:
> If I close() the mount FD "mfd", and then do "mount --move . /mnt", my
> printk() shows MNT_UMOUNT has been set. ( I guess fchdir() works more like
> openat(... , O_PATH) than dup() ). Then unmounting /mnt hangs, as I would
> expect from my previous test.
Okay, I think the
Okay, I put in a tracepoint (patch attached) and got a trace from the life of
the offending mount object. I've cropped non-useful information out of the
lines, inserted a blank line every time the usage count goes down to 2 and
dropped most of the lines generated by fsnotify.
Most of the lines
Okay, I put in a tracepoint (patch attached) and got a trace from the life of
the offending mount object. I've cropped non-useful information out of the
lines, inserted a blank line every time the usage count goes down to 2 and
dropped most of the lines generated by fsnotify.
Most of the lines
David Howells wrote:
> but this fails on your modified test-fsmount with:
>
> shell-init: error retrieving current directory: getcwd: cannot access
> parent directories: No such file or directory
Actually, it doesn't fail at this point, and I do see a splat later in
David Howells wrote:
> but this fails on your modified test-fsmount with:
>
> shell-init: error retrieving current directory: getcwd: cannot access
> parent directories: No such file or directory
Actually, it doesn't fail at this point, and I do see a splat later in
Alan Jenkins wrote:
> If I instead do the mount+unmount first, and close the FD as a second step, I
> think there's a lockup in the close(). The lockup happens in the same place
> as the unmount lockup from before. (Except there's a line "Code: Bad RIP
> value", I don't know why that happens).
Alan Jenkins wrote:
> If I instead do the mount+unmount first, and close the FD as a second step, I
> think there's a lockup in the close(). The lockup happens in the same place
> as the unmount lockup from before. (Except there's a line "Code: Bad RIP
> value", I don't know why that happens).
Hi David. I think there's an outstanding point below, have you been
thinking about it?
On 07/10/2018 11:48, Alan Jenkins wrote:
On 05/10/2018 19:24, Alan Jenkins wrote:
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE)
Hi David. I think there's an outstanding point below, have you been
thinking about it?
On 07/10/2018 11:48, Alan Jenkins wrote:
On 05/10/2018 19:24, Alan Jenkins wrote:
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE)
On Thu, Oct 11, 2018 at 09:17:54PM +0100, David Howells wrote:
> +/*
> + * Object if there are any nsfs mounts in the specified subtree. These can
> act
> + * as pins for mount namespaces that aren't checked by the mount-cycle
> checking
> + * code, thereby allowing cycles to be made.
> + */
>
On Thu, Oct 11, 2018 at 09:17:54PM +0100, David Howells wrote:
> +/*
> + * Object if there are any nsfs mounts in the specified subtree. These can
> act
> + * as pins for mount namespaces that aren't checked by the mount-cycle
> checking
> + * code, thereby allowing cycles to be made.
> + */
>
On 12/10/2018 15:54, David Howells wrote:
Alan Jenkins wrote:
+ open_tree_clone \
+ move_mount \
I'll rename them to test-XXX if you're okay with that.
David
Yes, that's fine.
Feel free to make adaptations you like. I don't have anything planned
for them myself, outside of
On 12/10/2018 15:54, David Howells wrote:
Alan Jenkins wrote:
+ open_tree_clone \
+ move_mount \
I'll rename them to test-XXX if you're okay with that.
David
Yes, that's fine.
Feel free to make adaptations you like. I don't have anything planned
for them myself, outside of
Alan Jenkins wrote:
> + open_tree_clone \
> + move_mount \
I'll rename them to test-XXX if you're okay with that.
David
Alan Jenkins wrote:
> + open_tree_clone \
> + move_mount \
I'll rename them to test-XXX if you're okay with that.
David
On 10/10/2018 13:36, David Howells wrote:
Alan Jenkins wrote:
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowe...@redhat.com)
Do you want to update that and I can take them into my patchset?
David
Sure :). I've attached a slightly updated
On 10/10/2018 13:36, David Howells wrote:
Alan Jenkins wrote:
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowe...@redhat.com)
Do you want to update that and I can take them into my patchset?
David
Sure :). I've attached a slightly updated
Eric W. Biederman wrote:
> It should just be a matter of replacing the test
> "if (p->mnt.mnt_sb->s_type == )" with "if mnt_ns_loop(p->mnt.mnt_root)"
Okay, the attached seems to work.
Thanks,
David
---
diff --git a/fs/namespace.c b/fs/namespace.c
index e969ded7d54b..5548fb9b7de2 100644
---
Eric W. Biederman wrote:
> It should just be a matter of replacing the test
> "if (p->mnt.mnt_sb->s_type == )" with "if mnt_ns_loop(p->mnt.mnt_root)"
Okay, the attached seems to work.
Thanks,
David
---
diff --git a/fs/namespace.c b/fs/namespace.c
index e969ded7d54b..5548fb9b7de2 100644
---
David Howells writes:
> Okay, this appears to fix the cycle-creation problem.
>
> It could probably be improved by comparing sequence numbers as Alan suggests,
> but I need to work out how to get at that.
It should just be a matter of replacing the test
"if (p->mnt.mnt_sb->s_type == )" with "if
David Howells writes:
> Okay, this appears to fix the cycle-creation problem.
>
> It could probably be improved by comparing sequence numbers as Alan suggests,
> but I need to work out how to get at that.
It should just be a matter of replacing the test
"if (p->mnt.mnt_sb->s_type == )" with "if
Okay, this appears to fix the cycle-creation problem.
It could probably be improved by comparing sequence numbers as Alan suggests,
but I need to work out how to get at that.
David
---
commit 069c3376f7849044117c866aeafbb1a525f84926
Author: David Howells
Date: Thu Oct 4 23:18:59 2018 +0100
Okay, this appears to fix the cycle-creation problem.
It could probably be improved by comparing sequence numbers as Alan suggests,
but I need to work out how to get at that.
David
---
commit 069c3376f7849044117c866aeafbb1a525f84926
Author: David Howells
Date: Thu Oct 4 23:18:59 2018 +0100
Alan Jenkins wrote:
> It sounds like you're under-estimating how we can use mnt_ns->seq (as is
> currently used in mnt_ns_loop()). Or maybe I am over-estimating it :).
I don't see how it helps. The duplication and attachment of the nsfs object
is already done by open_tree(), but as it's a
Alan Jenkins wrote:
> It sounds like you're under-estimating how we can use mnt_ns->seq (as is
> currently used in mnt_ns_loop()). Or maybe I am over-estimating it :).
I don't see how it helps. The duplication and attachment of the nsfs object
is already done by open_tree(), but as it's a
On 11/10/2018 13:14, David Howells wrote:
David Howells wrote:
The reason that you can do this with open_tree()/move_mount() is that it
allows you to create a mount tree (OPEN_TREE_CLONE) that has no namespace
assignment, pass it through the namespace switch and then attach it inside the
On 11/10/2018 13:14, David Howells wrote:
David Howells wrote:
The reason that you can do this with open_tree()/move_mount() is that it
allows you to create a mount tree (OPEN_TREE_CLONE) that has no namespace
assignment, pass it through the namespace switch and then attach it inside the
David Howells wrote:
> The reason that you can do this with open_tree()/move_mount() is that it
> allows you to create a mount tree (OPEN_TREE_CLONE) that has no namespace
> assignment, pass it through the namespace switch and then attach it inside the
> child namespace. The cross-namespace
David Howells wrote:
> The reason that you can do this with open_tree()/move_mount() is that it
> allows you to create a mount tree (OPEN_TREE_CLONE) that has no namespace
> assignment, pass it through the namespace switch and then attach it inside the
> child namespace. The cross-namespace
On 11/10/2018 10:17, David Howells wrote:
Alan Jenkins wrote:
# unshare --mount=private_mnt/child_ns --propagation=shared ls -l
/proc/self/ns/mnt
I think the problem is that the mount of the nsfs object done by unshare here
pins the new mount namespace - but doesn't add the namespace's
On 11/10/2018 10:17, David Howells wrote:
Alan Jenkins wrote:
# unshare --mount=private_mnt/child_ns --propagation=shared ls -l
/proc/self/ns/mnt
I think the problem is that the mount of the nsfs object done by unshare here
pins the new mount namespace - but doesn't add the namespace's
Alan Jenkins wrote:
> # unshare --mount=private_mnt/child_ns --propagation=shared ls -l
> /proc/self/ns/mnt
I think the problem is that the mount of the nsfs object done by unshare here
pins the new mount namespace - but doesn't add the namespace's contents into
the mount tree, so the mount
Alan Jenkins wrote:
> # unshare --mount=private_mnt/child_ns --propagation=shared ls -l
> /proc/self/ns/mnt
I think the problem is that the mount of the nsfs object done by unshare here
pins the new mount namespace - but doesn't add the namespace's contents into
the mount tree, so the mount
On 10/10/2018 14:02, David Howells wrote:
The attached change seems to fix the lazy-umount problem.
David
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 5adeeea2a4d9..d43f0fa152e9 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2472,7 +2472,7 @@ static int do_move_mount(struct path
On 10/10/2018 14:02, David Howells wrote:
The attached change seems to fix the lazy-umount problem.
David
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 5adeeea2a4d9..d43f0fa152e9 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2472,7 +2472,7 @@ static int do_move_mount(struct path
The attached change seems to fix the lazy-umount problem.
David
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 5adeeea2a4d9..d43f0fa152e9 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2472,7 +2472,7 @@ static int do_move_mount(struct path *old_path, struct
path *new_path)
The attached change seems to fix the lazy-umount problem.
David
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 5adeeea2a4d9..d43f0fa152e9 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2472,7 +2472,7 @@ static int do_move_mount(struct path *old_path, struct
path *new_path)
Alan Jenkins wrote:
> +pr_info("mnt_flags=%x umount=%x\n",
> + (unsigned) old->mnt.mnt_flags,
> + (unsigned) !!(old->mnt.mnt_flags & MNT_UMOUNT);
> +
Note that this doesn't actually compile, for want of a bracket.
David
Alan Jenkins wrote:
> +pr_info("mnt_flags=%x umount=%x\n",
> + (unsigned) old->mnt.mnt_flags,
> + (unsigned) !!(old->mnt.mnt_flags & MNT_UMOUNT);
> +
Note that this doesn't actually compile, for want of a bracket.
David
On 10/10/2018 13:31, David Howells wrote:
Alan Jenkins wrote:
# mount --move . /mnt
is this calling move_mount(2) on your system?
David
No. That was an unpatched mount program, from util-linux.
Alan
On 10/10/2018 13:31, David Howells wrote:
Alan Jenkins wrote:
# mount --move . /mnt
is this calling move_mount(2) on your system?
David
No. That was an unpatched mount program, from util-linux.
Alan
Alan Jenkins wrote:
> + * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowe...@redhat.com)
Do you want to update that and I can take them into my patchset?
David
Alan Jenkins wrote:
> + * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowe...@redhat.com)
Do you want to update that and I can take them into my patchset?
David
Alan Jenkins wrote:
> # mount --move . /mnt
is this calling move_mount(2) on your system?
David
Alan Jenkins wrote:
> # mount --move . /mnt
is this calling move_mount(2) on your system?
David
Alan Jenkins wrote:
> I replied last time to wonder about the MNT_UMOUNT mnt_flag. So I've tested it
> now :-), on David's current tree (commit 5581f4935add).
>
> The modified do_move_mount() allows re-attaching something that was
> lazy-unmounted. But the lazy unmount sets MNT_UMOUNT. And this
Alan Jenkins wrote:
> I replied last time to wonder about the MNT_UMOUNT mnt_flag. So I've tested it
> now :-), on David's current tree (commit 5581f4935add).
>
> The modified do_move_mount() allows re-attaching something that was
> lazy-unmounted. But the lazy unmount sets MNT_UMOUNT. And this
On 07/10/2018 11:48, Alan Jenkins wrote:
On 05/10/2018 19:24, Alan Jenkins wrote:
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of
On 07/10/2018 11:48, Alan Jenkins wrote:
On 05/10/2018 19:24, Alan Jenkins wrote:
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of
On 05/10/2018 19:24, Alan Jenkins wrote:
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its
tree is
not detached anymore,
On 05/10/2018 19:24, Alan Jenkins wrote:
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its
tree is
not detached anymore,
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
not detached anymore, it won't be dissolved. move_mount(2) is
On 21/09/2018 17:30, David Howells wrote:
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
not detached anymore, it won't be dissolved. move_mount(2) is
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
not detached anymore, it won't be dissolved. move_mount(2) is adjusted
to handle detached source.
That
From: Al Viro
Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).
If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
not detached anymore, it won't be dissolved. move_mount(2) is adjusted
to handle detached source.
That
80 matches
Mail list logo
