[CRIU] Re: [PATCH 2/4] sockets: Restore unconnected dgram sockets
Pavel Emelyanov
xemul at parallels.com
Mon Apr 16 06:05:33 EDT 2012
On 04/16/2012 01:27 PM, Cyrill Gorcunov wrote:
> On Mon, Apr 16, 2012 at 10:11:33AM +0400, Cyrill Gorcunov wrote:
>> On Mon, Apr 16, 2012 at 10:07:41AM +0400, Pavel Emelyanov wrote:
>>>> @@ -478,6 +481,7 @@ usage:
>>>> pr_msg(" -s leave tasks in stopped state after checkpoint instead of killing them\n");
>>>> pr_msg(" -n checkpoint/restore namespaces - values must be separated by comma\n");
>>>> pr_msg(" supported: uts, ipc\n");
>>>> + pr_msg(" -r resolve unconnected socket peer connection by name\n");
>>>
>>> I'd make the option meaning "allow external unix connections".
>>>
>>
>> OK. Maybe some another letter here, not 'r' but say 'x'?
>>
>
> What about below one? (Please don't pick it up though, it conflicts
> with current @master so I need to rebase before sending a final version)
>
> Cyrill
> ---
> From: Cyrill Gorcunov <gorcunov at openvz.org>
> Subject: [PATCH] sockets: Restore unconnected dgram sockets v2
>
> In case if dgram socket peer is not connected back
> we can try to resolve peer by name and connect there.
>
> For security reason this happens only if '-r' option
> is passed at restore time.
>
> In particular this is needed for crond checkpoint/restore,
> since it uses dgram socket to send messages to /dev/log.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
> crtools.c | 8 ++++++--
> include/crtools.h | 2 ++
> sockets.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
> 3 files changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/crtools.c b/crtools.c
> index 9f6b415..5229c1e 100644
> --- a/crtools.c
> +++ b/crtools.c
> @@ -23,7 +23,7 @@
> #include "uts_ns.h"
> #include "ipc_ns.h"
>
> -static struct cr_options opts;
> +struct cr_options opts;
>
> /*
> * The cr fd set is the set of files where the information
> @@ -336,7 +336,7 @@ int main(int argc, char *argv[])
> int log_inited = 0;
> int log_level = 0;
>
> - static const char short_opts[] = "dsf:p:t:hcD:o:n:v";
> + static const char short_opts[] = "dsf:p:t:hcD:o:n:vx";
>
> BUILD_BUG_ON(PAGE_SIZE != PAGE_IMAGE_SIZE);
>
> @@ -354,6 +354,9 @@ int main(int argc, char *argv[])
> case 's':
> opts.final_state = TASK_STOPPED;
> break;
> + case 'x':
> + opts.allow_external_unix_connections = true;
> + break;
> case 'p':
> pid = atoi(optarg);
> opts.leader_only = true;
> @@ -478,6 +481,7 @@ usage:
> pr_msg(" -s leave tasks in stopped state after checkpoint instead of killing them\n");
> pr_msg(" -n checkpoint/restore namespaces - values must be separated by comma\n");
> pr_msg(" supported: uts, ipc\n");
> + pr_msg(" -x allow external unix connections\n");
>
> pr_msg("\nAdditional common parameters:\n");
> pr_msg(" -D dir specifis directory where checkpoint files are/to be located\n");
> diff --git a/include/crtools.h b/include/crtools.h
> index d79878f..77bd8cf 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -67,6 +67,7 @@ struct cr_options {
> bool leader_only;
> bool show_pages_content;
> bool restore_detach;
> + bool allow_external_unix_connections;
> unsigned int namespaces_flags;
> };
>
> @@ -160,6 +161,7 @@ static inline int fdset_fd(const struct cr_fdset *fdset, int type)
> }
>
> extern struct cr_fdset *glob_fdset;
> +extern struct cr_options opts;
>
> int cr_dump_tasks(pid_t pid, const struct cr_options *opts);
> int cr_restore_tasks(pid_t pid, struct cr_options *opts);
> diff --git a/sockets.c b/sockets.c
> index 4825821..d960c9c 100644
> --- a/sockets.c
> +++ b/sockets.c
> @@ -38,6 +38,8 @@ static char buf[4096];
> #define SOCKFS_MAGIC 0x534F434B
> #endif
>
> +#define SOCK_STATE_XTRN 255
> +
> struct socket_desc {
> unsigned int family;
> unsigned int ino;
> @@ -412,10 +414,36 @@ static int dump_one_unix(const struct socket_desc *_sk, struct fd_parms *p,
> * Peer should have us as peer or have a name by which
> * we can access one.
> */
> - if (!peer->name && (peer->peer_ino != ue.id)) {
> - pr_err("Unix socket %x with unreachable peer %x (%x/%s)\n",
> - ue.id, ue.peer, peer->peer_ino, peer->name);
> - goto err;
> + if (peer->peer_ino != ue.id) {
> + if (!peer->name) {
> + pr_err("Unix socket %x with unreachable peer %x (%x/%s)\n",
> + ue.id, ue.peer, peer->peer_ino, peer->name);
> + goto err;
> + }
> +
> + struct unix_sk_entry pt = { };
> +
> + /*
> + * Socket is not connected back but have a name,
> + * so at restore time we will find it here. Note
> + * the id is set to impossible value by purpose.
> + */
> +
> + pt.id = peer->sd.ino;
> + pt.peer = peer->peer_ino;
pr.peer =<should be>= -1;
> + pt.type = SOCK_DGRAM;
> + pt.state = SOCK_STATE_XTRN;
TCP_LISTEN
"Externness" should be encoded in flags (yes, we should return them back on the
unix sk image.
> + pt.namelen = peer->namelen;
> +
> + show_one_unix("Dumping", peer);
> +
> + if (write_img(fdset_fd(glob_fdset, CR_FD_UNIXSK), &pt))
> + goto err;
> + if (write_img_buf(fdset_fd(glob_fdset, CR_FD_UNIXSK),
> + peer->name, pt.namelen))
> + goto err;
> +
> + show_one_unix_img("Dumped", &pt);
> }
> } else if (ue.state == TCP_ESTABLISHED) {
> const struct unix_sk_listen_icon *e;
> @@ -1076,6 +1104,8 @@ static inline char *skstate2s(u32 state)
> return "closed";
> else if (state == TCP_LISTEN)
> return "listen";
> + else if (state == SOCK_STATE_XTRN)
> + return "extern";
> else
> return unknown(state);
> }
> @@ -1212,6 +1242,9 @@ int run_unix_connections(void)
> struct fdinfo_list_entry *fle;
> struct sockaddr_un addr;
>
> + if (peer->ue.state == SOCK_STATE_XTRN)
> + goto next;
How did it happen to be here???
> +
> pr_info("\tConnect %x to %x\n", ui->ue.id, peer->ue.id);
>
> fle = file_master(&ui->d);
> @@ -1239,6 +1272,7 @@ try_again:
> if (set_fd_flags(fle->fd, ui->ue.flags))
> return -1;
>
> +next:
> cj = cj->next;
> }
>
> @@ -1484,9 +1518,12 @@ int resolve_unix_peers(void)
> continue;
>
> peer = find_unix_sk(ui->ue.peer);
> - if (!peer) {
> + pr_debug("peer %p\n", peer);
> + if (!peer ||
> + (peer && peer->ue.state == SOCK_STATE_XTRN &&
> + !opts.allow_external_unix_connections)) {
This one is OK.
> pr_err("FATAL: Peer %x unresolved for %x\n",
> - ui->ue.peer, ui->ue.id);
> + ui->ue.peer, ui->ue.id);
> return -1;
> }
>
More information about the CRIU
mailing list