[1/2] page-server: Fix incompatibility of page server protocol

Submitted by Pavel Emelianov on July 14, 2017, 3:30 p.m.

Details

Message ID 21054010-dd39-7be1-4611-c83525524ece@virtuozzo.com
State Accepted
Series "Series without cover letter"
Headers show

Commit Message

Pavel Emelianov July 14, 2017, 3:30 p.m.
It turned out (surprise surprise) that page server exchanges CR_FD_PAGEMAP
and CR_FD_SHMEM_PAGEMAP values in ps_iov_msg. The problem is that these
constants sit in enum and change their values from version to version %)

So here's the fixed version of the protocol including the backward compat
checks on all the values that could be met from older CRIUs (we're lucky
and they didn't intersect).

Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
---
 criu/page-xfer.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/criu/page-xfer.c b/criu/page-xfer.c
index d949c06..56d1853 100644
--- a/criu/page-xfer.c
+++ b/criu/page-xfer.c
@@ -54,14 +54,61 @@  static void psi2iovec(struct page_server_iov *ps, struct iovec *iov)
 #define PS_TYPE_BITS	8
 #define PS_TYPE_MASK	((1 << PS_TYPE_BITS) - 1)
 
+#define PS_TYPE_PID	(1)
+#define PS_TYPE_SHMEM	(2)
+/*
+ * XXX: When adding new types here check decode_pm_type for legacy
+ * numbers that can be met from older CRIUs
+ */
+
 static inline u64 encode_pm_id(int type, long id)
 {
+	if (type == CR_FD_PAGEMAP)
+		type = PS_TYPE_PID;
+	else if (type == CR_FD_SHMEM_PAGEMAP)
+		type = PS_TYPE_SHMEM;
+	else {
+		BUG();
+		return 0;
+	}
+
 	return ((u64)id) << PS_TYPE_BITS | type;
 }
 
 static int decode_pm_type(u64 dst_id)
 {
-	return dst_id & PS_TYPE_MASK;
+	int type;
+
+	/*
+	 * Magic numbers below came from the older CRIU versions that
+	 * errorneously used the changing CR_FD_* constants. The
+	 * changes were made when we merged images together and moved
+	 * the CR_FD_-s at the tail of the enum
+	 */
+	type = dst_id & PS_TYPE_MASK;
+	switch (type) {
+	case 10: /* 3.1 3.2 */
+	case 11: /* 1.3 1.4 1.5 1.6 1.7 1.8 2.* 3.0 */
+	case 16: /* 1.2 */
+	case 17: /* 1.0 1.1 */
+	case PS_TYPE_PID:
+		type = CR_FD_PAGEMAP;
+		break;
+	case 27: /* 1.3 */
+	case 28: /* 1.4 1.5 */
+	case 29: /* 1.6 1.7 */
+	case 32: /* 1.2 1.8 */
+	case 33: /* 1.0 1.1 3.1 3.2 */
+	case 34: /* 2.* 3.0 */
+	case PS_TYPE_SHMEM:
+		type = CR_FD_SHMEM_PAGEMAP;
+		break;
+	default:
+		type = -1;
+		break;
+	}
+
+	return type;
 }
 
 static long decode_pm_id(u64 dst_id)
@@ -497,6 +544,11 @@  static int page_server_check_parent(int sk, struct page_server_iov *pi)
 	type = decode_pm_type(pi->dst_id);
 	id = decode_pm_id(pi->dst_id);
 
+	if (type == -1) {
+		pr_err("Unknown pagemap type received\n");
+		return -1;
+	}
+
 	ret = check_parent_local_xfer(type, id);
 	if (ret < 0)
 		return -1;
@@ -571,6 +623,11 @@  static int page_server_open(int sk, struct page_server_iov *pi)
 
 	type = decode_pm_type(pi->dst_id);
 	id = decode_pm_id(pi->dst_id);
+	if (type == -1) {
+		pr_err("Unknown pagemap type received\n");
+		return -1;
+	}
+
 	pr_info("Opening %d/%ld\n", type, id);
 
 	page_server_close();

Comments

Pavel Emelianov July 17, 2017, 11:12 a.m.
On 07/14/2017 11:49 PM, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/2] page-server: Fix incompatibility of page server protocol
> URL   : https://patchwork.criu.org/series/1770/
> State : failure
> 
> == Logs ==
> 
> For more details see: https://travis-ci.org/criupatchwork/criu/builds/253687743?utm_source=github_status&utm_medium=notification
> .
> 
W: Failed to fetch http://security.ubuntu.com/ubuntu/dists/trusty-security/main/binary-i386/Packages  Hash Sum mismatch

W: Failed to fetch http://security.ubuntu.com/ubuntu/dists/trusty-security/universe/binary-i386/Packages  Hash Sum mismatch

on a single build.
Andrey Vagin July 29, 2017, 5:09 a.m.
Applied, thanks!
On Fri, Jul 14, 2017 at 06:30:57PM +0300, Pavel Emelyanov wrote:
> It turned out (surprise surprise) that page server exchanges CR_FD_PAGEMAP
> and CR_FD_SHMEM_PAGEMAP values in ps_iov_msg. The problem is that these
> constants sit in enum and change their values from version to version %)
> 
> So here's the fixed version of the protocol including the backward compat
> checks on all the values that could be met from older CRIUs (we're lucky
> and they didn't intersect).
> 
> Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
> ---
>  criu/page-xfer.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/page-xfer.c b/criu/page-xfer.c
> index d949c06..56d1853 100644
> --- a/criu/page-xfer.c
> +++ b/criu/page-xfer.c
> @@ -54,14 +54,61 @@ static void psi2iovec(struct page_server_iov *ps, struct iovec *iov)
>  #define PS_TYPE_BITS	8
>  #define PS_TYPE_MASK	((1 << PS_TYPE_BITS) - 1)
>  
> +#define PS_TYPE_PID	(1)
> +#define PS_TYPE_SHMEM	(2)
> +/*
> + * XXX: When adding new types here check decode_pm_type for legacy
> + * numbers that can be met from older CRIUs
> + */
> +
>  static inline u64 encode_pm_id(int type, long id)
>  {
> +	if (type == CR_FD_PAGEMAP)
> +		type = PS_TYPE_PID;
> +	else if (type == CR_FD_SHMEM_PAGEMAP)
> +		type = PS_TYPE_SHMEM;
> +	else {
> +		BUG();
> +		return 0;
> +	}
> +
>  	return ((u64)id) << PS_TYPE_BITS | type;
>  }
>  
>  static int decode_pm_type(u64 dst_id)
>  {
> -	return dst_id & PS_TYPE_MASK;
> +	int type;
> +
> +	/*
> +	 * Magic numbers below came from the older CRIU versions that
> +	 * errorneously used the changing CR_FD_* constants. The
> +	 * changes were made when we merged images together and moved
> +	 * the CR_FD_-s at the tail of the enum
> +	 */
> +	type = dst_id & PS_TYPE_MASK;
> +	switch (type) {
> +	case 10: /* 3.1 3.2 */
> +	case 11: /* 1.3 1.4 1.5 1.6 1.7 1.8 2.* 3.0 */
> +	case 16: /* 1.2 */
> +	case 17: /* 1.0 1.1 */
> +	case PS_TYPE_PID:
> +		type = CR_FD_PAGEMAP;
> +		break;
> +	case 27: /* 1.3 */
> +	case 28: /* 1.4 1.5 */
> +	case 29: /* 1.6 1.7 */
> +	case 32: /* 1.2 1.8 */
> +	case 33: /* 1.0 1.1 3.1 3.2 */
> +	case 34: /* 2.* 3.0 */
> +	case PS_TYPE_SHMEM:
> +		type = CR_FD_SHMEM_PAGEMAP;
> +		break;
> +	default:
> +		type = -1;
> +		break;
> +	}
> +
> +	return type;
>  }
>  
>  static long decode_pm_id(u64 dst_id)
> @@ -497,6 +544,11 @@ static int page_server_check_parent(int sk, struct page_server_iov *pi)
>  	type = decode_pm_type(pi->dst_id);
>  	id = decode_pm_id(pi->dst_id);
>  
> +	if (type == -1) {
> +		pr_err("Unknown pagemap type received\n");
> +		return -1;
> +	}
> +
>  	ret = check_parent_local_xfer(type, id);
>  	if (ret < 0)
>  		return -1;
> @@ -571,6 +623,11 @@ static int page_server_open(int sk, struct page_server_iov *pi)
>  
>  	type = decode_pm_type(pi->dst_id);
>  	id = decode_pm_id(pi->dst_id);
> +	if (type == -1) {
> +		pr_err("Unknown pagemap type received\n");
> +		return -1;
> +	}
> +
>  	pr_info("Opening %d/%ld\n", type, id);
>  
>  	page_server_close();
> -- 
> 2.1.4
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu