Dan Smith wrote:
> Avoid the leak reported by Serge for sockets that don't get attached to
> a file. Do this by orphaning the socket on restore if it is marked as
> SOCK_DEAD. If it is needed later for sending a buffer, temporarily
> adopt it for that process.
>
> Signed-off-by: Dan Smith <danms@xxxxxxxxxx>
This works well for the case of successful restart, but I suspect it
doesn't cover two other cases:
1) Malicious user removes the SOCK_DEAD marking off a socket.
2) Restart fails after a socket is restored but before it is attached
to a process.
In both cases, the sk->sk_socket of a non-dead socket isn't freed.
Note that we could detect the inconsistency of case 1 at the end of
restart, if there are non-attached sockets that aren't dead. But I
doubt if it makes any difference, because it is harmless (assuming
that the cleanup is indeed fixed).
Oren.
> ---
> net/checkpoint.c | 13 ++++++++++++-
> net/unix/checkpoint.c | 16 ++++++++++++++++
> 2 files changed, 28 insertions(+), 1 deletions(-)
>
> diff --git a/net/checkpoint.c b/net/checkpoint.c
> index d22341a..0bfca77 100644
> --- a/net/checkpoint.c
> +++ b/net/checkpoint.c
> @@ -627,6 +627,7 @@ struct sock *do_sock_restore(struct ckpt_ctx *ctx)
> {
> struct ckpt_hdr_socket *h;
> struct socket *sock;
> + struct sock *sk;
> int ret;
>
> h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_SOCKET);
> @@ -661,7 +662,14 @@ struct sock *do_sock_restore(struct ckpt_ctx *ctx)
>
> ckpt_hdr_put(ctx, h);
>
> - return sock->sk;
> + sk = sock->sk;
> + if (sock_flag(sk, SOCK_DEAD)) {
> + sock_orphan(sk);
> + sock->sk = NULL;
> + sock_release(sock);
> + }
> +
> + return sk;
> err:
> ckpt_hdr_put(ctx, h);
> sock_release(sock);
> @@ -688,6 +696,9 @@ struct file *sock_file_restore(struct ckpt_ctx *ctx, struct ckpt_hdr_file *ptr)
> if (IS_ERR(sk))
> return ERR_PTR(PTR_ERR(sk));
>
> + /* We should have a struct socket if we were attached to a file */
> + BUG_ON(!sk->sk_socket);
> +
> file = sock_alloc_attach_fd(sk->sk_socket);
> if (IS_ERR(file))
> return file;
> diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
> index 60f6a0c..da2b408 100644
> --- a/net/unix/checkpoint.c
> +++ b/net/unix/checkpoint.c
> @@ -180,6 +180,7 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx,
> struct kvec kvec;
> struct ckpt_hdr_socket_buffer *h;
> struct sock *sk;
> + struct socket *sock = NULL;
> uint8_t sock_shutdown;
> uint8_t peer_shutdown = 0;
> void *buf;
> @@ -229,6 +230,15 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx,
> ckpt_debug("Failed to join: %i\n", ret);
> goto out;
> }
> + } else
> +
> + if (!sk->sk_socket) {
> + ret = sock_create(sk->sk_family, sk->sk_type, 0, &sock);
> + if (ret < 0)
> + goto out;
> + if (unix_sk(sk)->peer)
> + sock->state = SS_CONNECTED;
> + sock_graft(sk, sock);
> }
>
> kvec.iov_len = len;
> @@ -275,6 +285,12 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx,
> ckpt_hdr_put(ctx, h);
> kfree(buf);
>
> + if (sock) {
> + sock_orphan(sk);
> + sock->sk = NULL;
> + sock_release(sock);
> + }
> +
> return ret;
> }
>
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
