[27/28] files: Add new master to file_desc if owners of existing fles have no permissions

Submitted by Kirill Tkhai on June 5, 2017, 5:27 p.m.

Details

Message ID 149668364423.25229.11850631182107931837.stgit@localhost.localdomain
State New
Series "Support sockets leaked to child user_ns task"
Headers show

Commit Message

Kirill Tkhai June 5, 2017, 5:27 p.m.
Iterate over fake_master_head and add a fake fake fle of root_item,
which becomes new master and have permissions to restore file_desc.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/cr-restore.c    |    5 +++++
 criu/files.c         |   31 +++++++++++++++++++++++++++++++
 criu/include/files.h |    1 +
 3 files changed, 37 insertions(+)

Patch hide | download patch | download mbox

diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index 88a9c50a5..9f69cd9bd 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -348,6 +348,11 @@  static int root_prepare_shared(void)
 	if (ret)
 		goto err;
 
+	/* This func may add new files, so it must be called before post prepare */
+	ret = add_fake_fds_masters();
+	if (ret)
+		goto err;
+
 	ret = run_post_prepare();
 	if (ret)
 		goto err;
diff --git a/criu/files.c b/criu/files.c
index b66217c02..0282b782c 100644
--- a/criu/files.c
+++ b/criu/files.c
@@ -1742,3 +1742,34 @@  int open_transport_socket(void)
 out:
 	return ret;
 }
+
+int add_fake_fds_masters(void)
+{
+	struct fdinfo_list_entry *fle;
+	struct file_desc *fdesc, *tmp;
+	FdinfoEntry *fe;
+	int fd;
+
+	list_for_each_entry_safe(fdesc, tmp, &fake_master_head, fake_master_list) {
+		fle = list_first_entry(&fdesc->fd_info_head,
+				       struct fdinfo_list_entry, desc_list);
+		/*
+		 * All masters are created in root_item for now.
+		 * Distribute them over pstree if someone reports,
+		 * their number is too big, or you want support
+		 * file->user_ns.
+		 */
+		fd = find_unused_fd(root_item, -1);
+		fe = dup_fdinfo(fle->fe, fd, fle->fe->flags);
+		if (!fe)
+			goto err;
+
+		if (collect_fd(vpid(root_item), fe, rsti(root_item), true))
+			goto err;
+	}
+	BUG_ON(!list_empty(&fake_master_head));
+	return 0;
+err:
+	pr_err("Can't prepare fds masters\n");
+	return -1;
+}
diff --git a/criu/include/files.h b/criu/include/files.h
index 812ef8947..6f651827b 100644
--- a/criu/include/files.h
+++ b/criu/include/files.h
@@ -151,6 +151,7 @@  extern int rst_file_params(int fd, FownEntry *fown, int flags);
 
 extern void show_saved_files(void);
 
+extern int add_fake_fds_masters(void);
 extern int prepare_fds(struct pstree_item *me);
 extern int prepare_fd_pid(struct pstree_item *me);
 extern int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id);

Comments

Andrei Vagin June 6, 2017, 7:17 p.m.
On Mon, Jun 05, 2017 at 08:27:24PM +0300, Kirill Tkhai wrote:
> Iterate over fake_master_head and add a fake fake fle of root_item,
> which becomes new master and have permissions to restore file_desc.
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  criu/cr-restore.c    |    5 +++++
>  criu/files.c         |   31 +++++++++++++++++++++++++++++++
>  criu/include/files.h |    1 +
>  3 files changed, 37 insertions(+)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index 88a9c50a5..9f69cd9bd 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -348,6 +348,11 @@ static int root_prepare_shared(void)
>  	if (ret)
>  		goto err;
>  
> +	/* This func may add new files, so it must be called before post prepare */
> +	ret = add_fake_fds_masters();
> +	if (ret)
> +		goto err;
> +
>  	ret = run_post_prepare();
>  	if (ret)
>  		goto err;
> diff --git a/criu/files.c b/criu/files.c
> index b66217c02..0282b782c 100644
> --- a/criu/files.c
> +++ b/criu/files.c
> @@ -1742,3 +1742,34 @@ int open_transport_socket(void)
>  out:
>  	return ret;
>  }
> +
> +int add_fake_fds_masters(void)
> +{
> +	struct fdinfo_list_entry *fle;
> +	struct file_desc *fdesc, *tmp;
> +	FdinfoEntry *fe;
> +	int fd;
> +
> +	list_for_each_entry_safe(fdesc, tmp, &fake_master_head, fake_master_list) {
> +		fle = list_first_entry(&fdesc->fd_info_head,
> +				       struct fdinfo_list_entry, desc_list);
> +		/*
> +		 * All masters are created in root_item for now.
> +		 * Distribute them over pstree if someone reports,
> +		 * their number is too big, or you want support
> +		 * file->user_ns.
> +		 */
> +		fd = find_unused_fd(root_item, -1);

why do we need to allocate a file descriptor for a fake master?

We need to open it and send to other processes, then we can close it.
Looks like this descriptor doesn't have to live long time.

> +		fe = dup_fdinfo(fle->fe, fd, fle->fe->flags);
> +		if (!fe)
> +			goto err;
> +
> +		if (collect_fd(vpid(root_item), fe, rsti(root_item), true))
> +			goto err;
> +	}
> +	BUG_ON(!list_empty(&fake_master_head));
> +	return 0;
> +err:
> +	pr_err("Can't prepare fds masters\n");
> +	return -1;
> +}
> diff --git a/criu/include/files.h b/criu/include/files.h
> index 812ef8947..6f651827b 100644
> --- a/criu/include/files.h
> +++ b/criu/include/files.h
> @@ -151,6 +151,7 @@ extern int rst_file_params(int fd, FownEntry *fown, int flags);
>  
>  extern void show_saved_files(void);
>  
> +extern int add_fake_fds_masters(void);
>  extern int prepare_fds(struct pstree_item *me);
>  extern int prepare_fd_pid(struct pstree_item *me);
>  extern int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id);
>
Kirill Tkhai June 6, 2017, 10:07 p.m.
On 06.06.2017 22:17, Andrei Vagin wrote:
> On Mon, Jun 05, 2017 at 08:27:24PM +0300, Kirill Tkhai wrote:
>> Iterate over fake_master_head and add a fake fake fle of root_item,
>> which becomes new master and have permissions to restore file_desc.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> ---
>>  criu/cr-restore.c    |    5 +++++
>>  criu/files.c         |   31 +++++++++++++++++++++++++++++++
>>  criu/include/files.h |    1 +
>>  3 files changed, 37 insertions(+)
>>
>> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
>> index 88a9c50a5..9f69cd9bd 100644
>> --- a/criu/cr-restore.c
>> +++ b/criu/cr-restore.c
>> @@ -348,6 +348,11 @@ static int root_prepare_shared(void)
>>  	if (ret)
>>  		goto err;
>>  
>> +	/* This func may add new files, so it must be called before post prepare */
>> +	ret = add_fake_fds_masters();
>> +	if (ret)
>> +		goto err;
>> +
>>  	ret = run_post_prepare();
>>  	if (ret)
>>  		goto err;
>> diff --git a/criu/files.c b/criu/files.c
>> index b66217c02..0282b782c 100644
>> --- a/criu/files.c
>> +++ b/criu/files.c
>> @@ -1742,3 +1742,34 @@ int open_transport_socket(void)
>>  out:
>>  	return ret;
>>  }
>> +
>> +int add_fake_fds_masters(void)
>> +{
>> +	struct fdinfo_list_entry *fle;
>> +	struct file_desc *fdesc, *tmp;
>> +	FdinfoEntry *fe;
>> +	int fd;
>> +
>> +	list_for_each_entry_safe(fdesc, tmp, &fake_master_head, fake_master_list) {
>> +		fle = list_first_entry(&fdesc->fd_info_head,
>> +				       struct fdinfo_list_entry, desc_list);
>> +		/*
>> +		 * All masters are created in root_item for now.
>> +		 * Distribute them over pstree if someone reports,
>> +		 * their number is too big, or you want support
>> +		 * file->user_ns.
>> +		 */
>> +		fd = find_unused_fd(root_item, -1);
> why do we need to allocate a file descriptor for a fake master?
>
> We need to open it and send to other processes, then we can close it.
> Looks like this descriptor doesn't have to live long time.

1)This allows to write code in generic way, and do not split it in many
particular cases.
Some file types use file_master() pid in their open() methods. Even if
we taught them,
we may have to re-teach them again, when something in file engine changes.
15-20 lines of generic code is better then the hell of particular cases.

2)Let's look at the future and check, that immediately closed fake
masters are also
a particular case. They are sent fles, but we'll have received fake fles
too. For example,
a closed fd, which is polled in process's epoll. How can we get it --
just add a fake
fle and the suggested engine will send the watched fle to the epoll's
owner task. No
engine changes are required.
The same for restoring msg_names of a unix socket messages, which have
different
senders.

No particular cases!

3)Suggested code is very simple. find_unused_fd() is fast as fles are
sorted, and the
only allocated memory is very low. Also I added only 2 bits to
fdinfo_list_entry -- if it's
a problem for our code, lets better group uint32_t structures members
everywhere!
(I suppose, one day we'll have to cleanup them all).


The only problem I see is the code iterates over all files when it's
looking for fake fles
to close. We may change this and to add fake fles in a separate list in
open_fdinfos()
like we already link completed fles to "completed" list (see the
function). Then iterate
only over it. How do you look on this?

>
>> +		fe = dup_fdinfo(fle->fe, fd, fle->fe->flags);
>> +		if (!fe)
>> +			goto err;
>> +
>> +		if (collect_fd(vpid(root_item), fe, rsti(root_item), true))
>> +			goto err;
>> +	}
>> +	BUG_ON(!list_empty(&fake_master_head));
>> +	return 0;
>> +err:
>> +	pr_err("Can't prepare fds masters\n");
>> +	return -1;
>> +}
>> diff --git a/criu/include/files.h b/criu/include/files.h
>> index 812ef8947..6f651827b 100644
>> --- a/criu/include/files.h
>> +++ b/criu/include/files.h
>> @@ -151,6 +151,7 @@ extern int rst_file_params(int fd, FownEntry *fown, int flags);
>>  
>>  extern void show_saved_files(void);
>>  
>> +extern int add_fake_fds_masters(void);
>>  extern int prepare_fds(struct pstree_item *me);
>>  extern int prepare_fd_pid(struct pstree_item *me);
>>  extern int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id);
>>