[4/5] mount: migrate bindmounts of external mounts

Submitted by Pavel Tikhomirov on March 1, 2017, 1:17 p.m.

Details

Message ID 20170301131745.22606-5-ptikhomirov@virtuozzo.com
State New
Series "mount: handle external mount bindmounts"
Headers show

Commit Message

Pavel Tikhomirov March 1, 2017, 1:17 p.m.
If container has external bindmount given to criu through
--ext-mount-map option by admin, container user can bindmount
subdirs of these external bindmount to somewhere else inside
container creating secondary external binmouns. Criu we will
fail to restore them as having unreachable sharing. But we can
restore secondary external bindmounts bindmounting them from
primary external bindmount.

https://jira.sw.ru/browse/PSBM-46753
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 criu/mount.c | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/mount.c b/criu/mount.c
index bd2497b..22a9c13 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -640,6 +640,35 @@  static bool does_mnt_overmount(struct mount_info *m)
 	return false;
 }
 
+/*
+ * Say we are external_bind if we are external or we
+ * will be bind from external, we set bind in propagate_mount
+ * and propagate_siblings
+ */
+
+static struct mount_info *external_bind(struct mount_info *m) {
+	struct mount_info *t;
+
+	if (m->external)
+		return m;
+
+	if (!list_empty(&m->mnt_share))
+		list_for_each_entry(t, &m->mnt_share, mnt_share)
+			if (t->external)
+				return t;
+
+	if (m->mnt_master)
+		return external_bind(m->mnt_master);
+
+	if (m->master_id > 0)
+		return NULL;
+	if (!list_empty(&m->mnt_bind))
+		list_for_each_entry(t, &m->mnt_bind, mnt_bind)
+			if (issubpath(m->root, t->root) && t->external)
+				return t;
+	return NULL;
+}
+
 static int validate_mounts(struct mount_info *info, bool for_dump)
 {
 	struct mount_info *m, *t;
@@ -652,7 +681,7 @@  static int validate_mounts(struct mount_info *info, bool for_dump)
 		if (m->shared_id && validate_shared(m))
 			return -1;
 
-		if (m->external)
+		if (external_bind(m))
 			goto skip_fstype;
 
 		/*
@@ -908,9 +937,9 @@  static int resolve_shared_mounts(struct mount_info *info, int root_master_id)
 
 		/*
 		 * If we haven't already determined this mount is external,
-		 * then we don't know where it came from.
+		 * or bind of external, then we don't know where it came from.
 		 */
-		if (need_master && m->parent && !m->external) {
+		if (need_master && m->parent && !external_bind(m)) {
 			pr_err("Mount %d %s (master_id: %d shared_id: %d) "
 			       "has unreachable sharing. Try --enable-external-masters.\n", m->mnt_id,
 				m->mountpoint, m->master_id, m->shared_id);
@@ -1222,7 +1251,7 @@  static int dump_one_fs(struct mount_info *mi)
 	struct mount_info *t;
 	bool first = true;
 
-	if (mi->is_ns_root || mi->need_plugin || mi->external || !mi->fstype->dump)
+	if (mi->is_ns_root || mi->need_plugin || external_bind(mi) || !mi->fstype->dump)
 		return 0;
 
 	/* mnt_bind is a cycled list, so list_for_each can't be used here. */
@@ -1634,7 +1663,7 @@  static int propagate_mount(struct mount_info *mi)
 	 * FIXME Currently non-root mounts can be restored
 	 * only if a proper root mount exists
 	 */
-	if (fsroot_mounted(mi) || mi->parent == root_yard_mp) {
+	if (fsroot_mounted(mi) || mi->parent == root_yard_mp || mi->external) {
 		list_for_each_entry(t, &mi->mnt_bind, mnt_bind) {
 			if (t->mounted)
 				continue;
@@ -1642,6 +1671,8 @@  static int propagate_mount(struct mount_info *mi)
 				continue;
 			if (t->master_id > 0)
 				continue;
+			if (!issubpath(t->root, mi->root))
+				continue;
 			t->bind = mi;
 			t->s_dev_rt = mi->s_dev_rt;
 		}

Comments

Andrey Vagin March 2, 2017, 12:02 a.m.
On Wed, Mar 01, 2017 at 04:17:44PM +0300, Pavel Tikhomirov wrote:
> If container has external bindmount given to criu through
> --ext-mount-map option by admin, container user can bindmount
> subdirs of these external bindmount to somewhere else inside
> container creating secondary external binmouns. Criu we will
> fail to restore them as having unreachable sharing. But we can
> restore secondary external bindmounts bindmounting them from
> primary external bindmount.
> 
> https://jira.sw.ru/browse/PSBM-46753
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  criu/mount.c | 41 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/criu/mount.c b/criu/mount.c
> index bd2497b..22a9c13 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -640,6 +640,35 @@ static bool does_mnt_overmount(struct mount_info *m)
>  	return false;
>  }
>  
> +/*
> + * Say we are external_bind if we are external or we
> + * will be bind from external, we set bind in propagate_mount
> + * and propagate_siblings
> + */
> +
> +static struct mount_info *external_bind(struct mount_info *m) {

Can we call this function as mnt_is_external? A return value isn't
used...

> +	struct mount_info *t;
> +
> +	if (m->external)
> +		return m;
> +
> +	if (!list_empty(&m->mnt_share))
> +		list_for_each_entry(t, &m->mnt_share, mnt_share)
> +			if (t->external)
> +				return t;
> +
> +	if (m->mnt_master)
> +		return external_bind(m->mnt_master);

We do we need to check mnt_master here?

> +
> +	if (m->master_id > 0)
> +		return NULL;
> +	if (!list_empty(&m->mnt_bind))
> +		list_for_each_entry(t, &m->mnt_bind, mnt_bind)
> +			if (issubpath(m->root, t->root) && t->external)
> +				return t;
> +	return NULL;
> +}
> +
>  static int validate_mounts(struct mount_info *info, bool for_dump)
>  {
>  	struct mount_info *m, *t;
> @@ -652,7 +681,7 @@ static int validate_mounts(struct mount_info *info, bool for_dump)
>  		if (m->shared_id && validate_shared(m))
>  			return -1;
>  
> -		if (m->external)
> +		if (external_bind(m))
>  			goto skip_fstype;
>  
>  		/*
> @@ -908,9 +937,9 @@ static int resolve_shared_mounts(struct mount_info *info, int root_master_id)
>  
>  		/*
>  		 * If we haven't already determined this mount is external,
> -		 * then we don't know where it came from.
> +		 * or bind of external, then we don't know where it came from.
>  		 */
> -		if (need_master && m->parent && !m->external) {
> +		if (need_master && m->parent && !external_bind(m)) {
>  			pr_err("Mount %d %s (master_id: %d shared_id: %d) "
>  			       "has unreachable sharing. Try --enable-external-masters.\n", m->mnt_id,
>  				m->mountpoint, m->master_id, m->shared_id);
> @@ -1222,7 +1251,7 @@ static int dump_one_fs(struct mount_info *mi)
>  	struct mount_info *t;
>  	bool first = true;
>  
> -	if (mi->is_ns_root || mi->need_plugin || mi->external || !mi->fstype->dump)
> +	if (mi->is_ns_root || mi->need_plugin || external_bind(mi) || !mi->fstype->dump)
>  		return 0;
>  
>  	/* mnt_bind is a cycled list, so list_for_each can't be used here. */
> @@ -1634,7 +1663,7 @@ static int propagate_mount(struct mount_info *mi)
>  	 * FIXME Currently non-root mounts can be restored
>  	 * only if a proper root mount exists
>  	 */
> -	if (fsroot_mounted(mi) || mi->parent == root_yard_mp) {
> +	if (fsroot_mounted(mi) || mi->parent == root_yard_mp || mi->external) {
>  		list_for_each_entry(t, &mi->mnt_bind, mnt_bind) {
>  			if (t->mounted)
>  				continue;
> @@ -1642,6 +1671,8 @@ static int propagate_mount(struct mount_info *mi)
>  				continue;
>  			if (t->master_id > 0)
>  				continue;
> +			if (!issubpath(t->root, mi->root))
> +				continue;
>  			t->bind = mi;
>  			t->s_dev_rt = mi->s_dev_rt;
>  		}
> -- 
> 2.9.3
>
Pavel Tikhomirov March 2, 2017, 6:26 a.m.
----Пользователь Andrew Vagin написал ----

> On Wed, Mar 01, 2017 at 04:17:44PM +0300, Pavel Tikhomirov wrote:
> > If container has external bindmount given to criu through
> > --ext-mount-map option by admin, container user can bindmount
> > subdirs of these external bindmount to somewhere else inside
> > container creating secondary external binmouns. Criu we will
> > fail to restore them as having unreachable sharing. But we can
> > restore secondary external bindmounts bindmounting them from
> > primary external bindmount.
> > 
> > https://jira.sw.ru/browse/PSBM-46753
> > Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> > ---
> >  criu/mount.c | 41 ++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 36 insertions(+), 5 deletions(-)
> > 
> > diff --git a/criu/mount.c b/criu/mount.c
> > index bd2497b..22a9c13 100644
> > --- a/criu/mount.c
> > +++ b/criu/mount.c
> > @@ -640,6 +640,35 @@ static bool does_mnt_overmount(struct mount_info *m)
> >  	return false;
> >  }
> >  
> > +/*
> > + * Say we are external_bind if we are external or we
> > + * will be bind from external, we set bind in propagate_mount
> > + * and propagate_siblings
> > + */
> > +
> > +static struct mount_info *external_bind(struct mount_info *m) {
> 
> Can we call this function as mnt_is_external? A return value isn't
> used...

Yes, will do boolean ret and rename.

> 
> > +	struct mount_info *t;
> > +
> > +	if (m->external)
> > +		return m;
> > +
> > +	if (!list_empty(&m->mnt_share))
> > +		list_for_each_entry(t, &m->mnt_share, mnt_share)
> > +			if (t->external)
> > +				return t;
> > +
> > +	if (m->mnt_master)
> > +		return external_bind(m->mnt_master);
> 
> We do we need to check mnt_master here?

Imagine we have non-root external mount, it's shared bind-mount and latter's slave bind-mount, so that criu thinks that mnt_master of the third is the second. In that case we will first restore external, next restore the second(shared) as external's bind and last the third(slave) as a bind of shared(through mount propagation). We just need to allow third mount, and to find out that it is allowed we need to go through all masters and search their shares.

> 
> > +
> > +	if (m->master_id > 0)
> > +		return NULL;
> > +	if (!list_empty(&m->mnt_bind))
> > +		list_for_each_entry(t, &m->mnt_bind, mnt_bind)
> > +			if (issubpath(m->root, t->root) && t->external)
> > +				return t;
> > +	return NULL;
> > +}
> > +
> >  static int validate_mounts(struct mount_info *info, bool for_dump)
> >  {
> >  	struct mount_info *m, *t;
> > @@ -652,7 +681,7 @@ static int validate_mounts(struct mount_info *info, bool for_dump)
> >  		if (m->shared_id && validate_shared(m))
> >  			return -1;
> >  
> > -		if (m->external)
> > +		if (external_bind(m))
> >  			goto skip_fstype;
> >  
> >  		/*
> > @@ -908,9 +937,9 @@ static int resolve_shared_mounts(struct mount_info *info, int root_master_id)
> >  
> >  		/*
> >  		 * If we haven't already determined this mount is external,
> > -		 * then we don't know where it came from.
> > +		 * or bind of external, then we don't know where it came from.
> >  		 */
> > -		if (need_master && m->parent && !m->external) {
> > +		if (need_master && m->parent && !external_bind(m)) {
> >  			pr_err("Mount %d %s (master_id: %d shared_id: %d) "
> >  			       "has unreachable sharing. Try --enable-external-masters.\n", m->mnt_id,
> >  				m->mountpoint, m->master_id, m->shared_id);
> > @@ -1222,7 +1251,7 @@ static int dump_one_fs(struct mount_info *mi)
> >  	struct mount_info *t;
> >  	bool first = true;
> >  
> > -	if (mi->is_ns_root || mi->need_plugin || mi->external || !mi->fstype->dump)
> > +	if (mi->is_ns_root || mi->need_plugin || external_bind(mi) || !mi->fstype->dump)
> >  		return 0;
> >  
> >  	/* mnt_bind is a cycled list, so list_for_each can't be used here. */
> > @@ -1634,7 +1663,7 @@ static int propagate_mount(struct mount_info *mi)
> >  	 * FIXME Currently non-root mounts can be restored
> >  	 * only if a proper root mount exists
> >  	 */
> > -	if (fsroot_mounted(mi) || mi->parent == root_yard_mp) {
> > +	if (fsroot_mounted(mi) || mi->parent == root_yard_mp || mi->external) {
> >  		list_for_each_entry(t, &mi->mnt_bind, mnt_bind) {
> >  			if (t->mounted)
> >  				continue;
> > @@ -1642,6 +1671,8 @@ static int propagate_mount(struct mount_info *mi)
> >  				continue;
> >  			if (t->master_id > 0)
> >  				continue;
> > +			if (!issubpath(t->root, mi->root))
> > +				continue;
> >  			t->bind = mi;
> >  			t->s_dev_rt = mi->s_dev_rt;
> >  		}
> > -- 
> > 2.9.3
> >
Andrey Vagin March 15, 2017, 10:09 p.m.
On Thu, Mar 02, 2017 at 09:26:40AM +0300, Pavel Tikhomirov wrote:
> ----Пользователь Andrew Vagin написал ----
> 
> > On Wed, Mar 01, 2017 at 04:17:44PM +0300, Pavel Tikhomirov wrote:
> > > If container has external bindmount given to criu through
> > > --ext-mount-map option by admin, container user can bindmount
> > > subdirs of these external bindmount to somewhere else inside
> > > container creating secondary external binmouns. Criu we will
> > > fail to restore them as having unreachable sharing. But we can
> > > restore secondary external bindmounts bindmounting them from
> > > primary external bindmount.
> > >
> > > https://jira.sw.ru/browse/PSBM-46753
> > > Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> > > ---
> > >  criu/mount.c | 41 ++++++++++++++++++++++++++++++++++++-----
> > >  1 file changed, 36 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/criu/mount.c b/criu/mount.c
> > > index bd2497b..22a9c13 100644
> > > --- a/criu/mount.c
> > > +++ b/criu/mount.c
> > > @@ -640,6 +640,35 @@ static bool does_mnt_overmount(struct mount_info *m)
> > >  return false;
> > >  }
> > > 
> > > +/*
> > > + * Say we are external_bind if we are external or we
> > > + * will be bind from external, we set bind in propagate_mount
> > > + * and propagate_siblings
> > > + */
> > > +
> > > +static struct mount_info *external_bind(struct mount_info *m) {
> >
> > Can we call this function as mnt_is_external? A return value isn't
> > used...
> 
> Yes, will do boolean ret and rename.
> 
> >
> > > + struct mount_info *t;
> > > +
> > > + if (m->external)
> > > + return m;
> > > +
> > > + if (!list_empty(&m->mnt_share))
> > > + list_for_each_entry(t, &m->mnt_share, mnt_share)
> > > + if (t->external)
> > > + return t;
> > > +
> > > + if (m->mnt_master)
> > > + return external_bind(m->mnt_master);
> >
> > We do we need to check mnt_master here?
> 
> Imagine we have non-root external mount, it's shared bind-mount and latter's
> slave bind-mount, so that criu thinks that mnt_master of the third is the
> second. In that case we will first restore external, next restore the second
> (shared) as external's bind and last the third(slave) as a bind of shared
> (through mount propagation). We just need to allow third mount, and to find out
> that it is allowed we need to go through all masters and search their shares.
> 

Will the test fail if we remove this check? If the answer is no, could
you extand the test? Thanks.

> >
> > > +
> > > + if (m->master_id > 0)
> > > + return NULL;
> > > + if (!list_empty(&m->mnt_bind))
> > > + list_for_each_entry(t, &m->mnt_bind, mnt_bind)
> > > + if (issubpath(m->root, t->root) && t->external)
> > > + return t;
> > > + return NULL;
> > > +}
> > > +
> > >  static int validate_mounts(struct mount_info *info, bool for_dump)
> > >  {
> > >  struct mount_info *m, *t;
> > > @@ -652,7 +681,7 @@ static int validate_mounts(struct mount_info *info,
> bool for_dump)
> > >  if (m->shared_id && validate_shared(m))
> > >  return -1;
> > > 
> > > - if (m->external)
> > > + if (external_bind(m))
> > >  goto skip_fstype;
> > > 
> > >  /*
> > > @@ -908,9 +937,9 @@ static int resolve_shared_mounts(struct mount_info
> *info, int root_master_id)
> > > 
> > >  /*
> > >  * If we haven't already determined this mount is external,
> > > - * then we don't know where it came from.
> > > + * or bind of external, then we don't know where it came from.
> > >  */
> > > - if (need_master && m->parent && !m->external) {
> > > + if (need_master && m->parent && !external_bind(m)) {
> > >  pr_err("Mount %d %s (master_id: %d shared_id: %d) "
> > >         "has unreachable sharing. Try --enable-external-masters.\n", m->
> mnt_id,
> > >  m->mountpoint, m->master_id, m->shared_id);
> > > @@ -1222,7 +1251,7 @@ static int dump_one_fs(struct mount_info *mi)
> > >  struct mount_info *t;
> > >  bool first = true;
> > > 
> > > - if (mi->is_ns_root || mi->need_plugin || mi->external || !mi->fstype->
> dump)
> > > + if (mi->is_ns_root || mi->need_plugin || external_bind(mi) || !mi->
> fstype->dump)
> > >  return 0;
> > > 
> > >  /* mnt_bind is a cycled list, so list_for_each can't be used here. */
> > > @@ -1634,7 +1663,7 @@ static int propagate_mount(struct mount_info *mi)
> > >  * FIXME Currently non-root mounts can be restored
> > >  * only if a proper root mount exists
> > >  */
> > > - if (fsroot_mounted(mi) || mi->parent == root_yard_mp) {
> > > + if (fsroot_mounted(mi) || mi->parent == root_yard_mp || mi->external) {
> > >  list_for_each_entry(t, &mi->mnt_bind, mnt_bind) {
> > >  if (t->mounted)
> > >  continue;
> > > @@ -1642,6 +1671,8 @@ static int propagate_mount(struct mount_info *mi)
> > >  continue;
> > >  if (t->master_id > 0)
> > >  continue;
> > > + if (!issubpath(t->root, mi->root))
> > > + continue;
> > >  t->bind = mi;
> > >  t->s_dev_rt = mi->s_dev_rt;
> > >  }
> > > --
> > > 2.9.3
> > >