[PATCH 0/3] xen-netback: hash mapping hanling adjustments

[PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Jan Beulich]

First and foremost the fix for XSA-270. On top of that further changes
which looked desirable to me while investigating that XSA.

1: fix input validation in xenvif_set_hash_mapping()
2: validate queue numbers in xenvif_set_hash_mapping()
3: handle page straddling in xenvif_set_hash_mapping()

Signed-off-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/3] xen-netback: fix input validation in xenvif_set_hash_mapping()

[Jan Beulich]

Both len and off are frontend specified values, so we need to make
sure there's no overflow when adding the two for the bounds check. We
also want to avoid undefined behavior and hence use off to index into
->hash.mapping[] only after bounds checking. This at the same time
allows to take care of not applying off twice for the bounds checking
against vif->num_queues.

It is also insufficient to bounds check copy_op.len, as this is len
truncated to 16 bits.

This is XSA-270 / CVE-2018-15471.

Reported-by: Felix Wilhelm <fwilhelm@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Tested-by: Paul Durrant <paul.durrant@citrix.com>
Cc: stable@vger.kernel.org [4.7 onwards]

---
 drivers/net/xen-netback/hash.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -332,20 +332,22 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = &vif->hash.mapping[off];
+	u32 *mapping = vif->hash.mapping;
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
-		.dest.u.gmfn = virt_to_gfn(mapping),
 		.dest.domid = DOMID_SELF,
-		.dest.offset = xen_offset_in_page(mapping),
-		.len = len * sizeof(u32),
+		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
 	};
 
-	if ((off + len > vif->hash.size) || copy_op.len > XEN_PAGE_SIZE)
+	if ((off + len < off) || (off + len > vif->hash.size) ||
+	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
+	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+
 	while (len-- != 0)
 		if (mapping[off++] >= vif->num_queues)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Jan Beulich]

Checking them before the grant copy means nothing as to the validity of
the incoming request. As we shouldn't make the new data live before
having validated it, introduce a second instance of the mapping array.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 drivers/net/xen-netback/common.h    |    3 ++-
 drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
 drivers/net/xen-netback/interface.c |    3 ++-
 3 files changed, 18 insertions(+), 8 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/common.h
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/common.h
@@ -241,8 +241,9 @@ struct xenvif_hash_cache {
 struct xenvif_hash {
 	unsigned int alg;
 	u32 flags;
+	bool mapping_sel;
 	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
-	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
+	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
 	unsigned int size;
 	struct xenvif_hash_cache cache;
 };
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
 	vif->hash.size = size;
-	memset(vif->hash.mapping, 0, sizeof(u32) * size);
+	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
+	       sizeof(u32) * size);
 
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
@@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = vif->hash.mapping;
+	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
@@ -348,9 +349,8 @@ u32 xenvif_set_hash_mapping(struct xenvi
 	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
 	copy_op.dest.offset = xen_offset_in_page(mapping + off);
 
-	while (len-- != 0)
-		if (mapping[off++] >= vif->num_queues)
-			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
+	       vif->hash.size * sizeof(*mapping));
 
 	if (copy_op.len != 0) {
 		gnttab_batch_copy(&copy_op, 1);
@@ -359,6 +359,12 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 
+	while (len-- != 0)
+		if (mapping[off++] >= vif->num_queues)
+			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+
+	vif->hash.mapping_sel = !vif->hash.mapping_sel;
+
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
 
@@ -410,6 +416,8 @@ void xenvif_dump_hash_info(struct xenvif
 	}
 
 	if (vif->hash.size != 0) {
+		const u32 *mapping = vif->hash.mapping[vif->hash.mapping_sel];
+
 		seq_puts(m, "\nHash Mapping:\n");
 
 		for (i = 0; i < vif->hash.size; ) {
@@ -422,7 +430,7 @@ void xenvif_dump_hash_info(struct xenvif
 			seq_printf(m, "[%4u - %4u]: ", i, i + n - 1);
 
 			for (j = 0; j < n; j++, i++)
-				seq_printf(m, "%4u ", vif->hash.mapping[i]);
+				seq_printf(m, "%4u ", mapping[i]);
 
 			seq_puts(m, "\n");
 		}
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/interface.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/interface.c
@@ -162,7 +162,8 @@ static u16 xenvif_select_queue(struct ne
 	if (size == 0)
 		return skb_get_hash_raw(skb) % dev->real_num_tx_queues;
 
-	return vif->hash.mapping[skb_get_hash_raw(skb) % size];
+	return vif->hash.mapping[vif->hash.mapping_sel]
+				[skb_get_hash_raw(skb) % size];
 }
 
 static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)

[PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Jan Beulich]

Checking them before the grant copy means nothing as to the validity of
the incoming request. As we shouldn't make the new data live before
having validated it, introduce a second instance of the mapping array.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 drivers/net/xen-netback/common.h    |    3 ++-
 drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
 drivers/net/xen-netback/interface.c |    3 ++-
 3 files changed, 18 insertions(+), 8 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/common.h
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/common.h
@@ -241,8 +241,9 @@ struct xenvif_hash_cache {
 struct xenvif_hash {
 	unsigned int alg;
 	u32 flags;
+	bool mapping_sel;
 	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
-	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
+	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
 	unsigned int size;
 	struct xenvif_hash_cache cache;
 };
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
 	vif->hash.size = size;
-	memset(vif->hash.mapping, 0, sizeof(u32) * size);
+	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
+	       sizeof(u32) * size);
 
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
@@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = vif->hash.mapping;
+	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
@@ -348,9 +349,8 @@ u32 xenvif_set_hash_mapping(struct xenvi
 	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
 	copy_op.dest.offset = xen_offset_in_page(mapping + off);
 
-	while (len-- != 0)
-		if (mapping[off++] >= vif->num_queues)
-			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
+	       vif->hash.size * sizeof(*mapping));
 
 	if (copy_op.len != 0) {
 		gnttab_batch_copy(&copy_op, 1);
@@ -359,6 +359,12 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 
+	while (len-- != 0)
+		if (mapping[off++] >= vif->num_queues)
+			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+
+	vif->hash.mapping_sel = !vif->hash.mapping_sel;
+
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
 
@@ -410,6 +416,8 @@ void xenvif_dump_hash_info(struct xenvif
 	}
 
 	if (vif->hash.size != 0) {
+		const u32 *mapping = vif->hash.mapping[vif->hash.mapping_sel];
+
 		seq_puts(m, "\nHash Mapping:\n");
 
 		for (i = 0; i < vif->hash.size; ) {
@@ -422,7 +430,7 @@ void xenvif_dump_hash_info(struct xenvif
 			seq_printf(m, "[%4u - %4u]: ", i, i + n - 1);
 
 			for (j = 0; j < n; j++, i++)
-				seq_printf(m, "%4u ", vif->hash.mapping[i]);
+				seq_printf(m, "%4u ", mapping[i]);
 
 			seq_puts(m, "\n");
 		}
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/interface.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/interface.c
@@ -162,7 +162,8 @@ static u16 xenvif_select_queue(struct ne
 	if (size == 0)
 		return skb_get_hash_raw(skb) % dev->real_num_tx_queues;
 
-	return vif->hash.mapping[skb_get_hash_raw(skb) % size];
+	return vif->hash.mapping[vif->hash.mapping_sel]
+				[skb_get_hash_raw(skb) % size];
 }
 
 static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/3] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Jan Beulich]

There's no guarantee that the mapping array doesn't cross a page
boundary. Use a second grant copy operation if necessary.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 drivers/net/xen-netback/hash.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -334,28 +334,39 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			    u32 off)
 {
 	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
-	struct gnttab_copy copy_op = {
+	unsigned int nr = 1;
+	struct gnttab_copy copy_op[2] = {{
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
 		.dest.domid = DOMID_SELF,
 		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
-	};
+	}};
 
 	if ((off + len < off) || (off + len > vif->hash.size) ||
 	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
-	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
-	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+	copy_op[0].dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op[0].dest.offset = xen_offset_in_page(mapping + off);
+	if (copy_op[0].dest.offset + copy_op[0].len > XEN_PAGE_SIZE) {
+		copy_op[1] = copy_op[0];
+		copy_op[1].source.offset = XEN_PAGE_SIZE - copy_op[0].dest.offset;
+		copy_op[1].dest.u.gmfn = virt_to_gfn(mapping + off + len);
+		copy_op[1].dest.offset = 0;
+		copy_op[1].len = copy_op[0].len - copy_op[1].source.offset;
+		copy_op[0].len = copy_op[1].source.offset;
+		nr = 2;
+	}
 
 	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
 	       vif->hash.size * sizeof(*mapping));
 
-	if (copy_op.len != 0) {
-		gnttab_batch_copy(&copy_op, 1);
+	if (copy_op[0].len != 0) {
+		gnttab_batch_copy(copy_op, nr);
 
-		if (copy_op.status != GNTST_okay)
+		if (copy_op[0].status != GNTST_okay ||
+		    copy_op[nr - 1].status != GNTST_okay)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 

[PATCH 3/3] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Jan Beulich]

There's no guarantee that the mapping array doesn't cross a page
boundary. Use a second grant copy operation if necessary.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 drivers/net/xen-netback/hash.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -334,28 +334,39 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			    u32 off)
 {
 	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
-	struct gnttab_copy copy_op = {
+	unsigned int nr = 1;
+	struct gnttab_copy copy_op[2] = {{
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
 		.dest.domid = DOMID_SELF,
 		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
-	};
+	}};
 
 	if ((off + len < off) || (off + len > vif->hash.size) ||
 	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
-	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
-	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+	copy_op[0].dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op[0].dest.offset = xen_offset_in_page(mapping + off);
+	if (copy_op[0].dest.offset + copy_op[0].len > XEN_PAGE_SIZE) {
+		copy_op[1] = copy_op[0];
+		copy_op[1].source.offset = XEN_PAGE_SIZE - copy_op[0].dest.offset;
+		copy_op[1].dest.u.gmfn = virt_to_gfn(mapping + off + len);
+		copy_op[1].dest.offset = 0;
+		copy_op[1].len = copy_op[0].len - copy_op[1].source.offset;
+		copy_op[0].len = copy_op[1].source.offset;
+		nr = 2;
+	}
 
 	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
 	       vif->hash.size * sizeof(*mapping));
 
-	if (copy_op.len != 0) {
-		gnttab_batch_copy(&copy_op, 1);
+	if (copy_op[0].len != 0) {
+		gnttab_batch_copy(copy_op, nr);
 
-		if (copy_op.status != GNTST_okay)
+		if (copy_op[0].status != GNTST_okay ||
+		    copy_op[nr - 1].status != GNTST_okay)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Wei Liu]

On Tue, Aug 28, 2018 at 08:59:45AM -0600, Jan Beulich wrote:
> Checking them before the grant copy means nothing as to the validity of
> the incoming request. As we shouldn't make the new data live before
> having validated it, introduce a second instance of the mapping array.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  drivers/net/xen-netback/common.h    |    3 ++-
>  drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
>  drivers/net/xen-netback/interface.c |    3 ++-
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/common.h
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/common.h
> @@ -241,8 +241,9 @@ struct xenvif_hash_cache {
>  struct xenvif_hash {
>  	unsigned int alg;
>  	u32 flags;
> +	bool mapping_sel;
>  	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
> -	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
> +	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
>  	unsigned int size;
>  	struct xenvif_hash_cache cache;
>  };
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
> @@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
>  		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
>  
>  	vif->hash.size = size;
> -	memset(vif->hash.mapping, 0, sizeof(u32) * size);
> +	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
> +	       sizeof(u32) * size);
>  
>  	return XEN_NETIF_CTRL_STATUS_SUCCESS;
>  }
> @@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
>  u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
>  			    u32 off)
>  {
> -	u32 *mapping = vif->hash.mapping;
> +	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];

Can you rename this to inactive_mapping so the code can be followed more
easily?

The code looks correct to me, but I would like Paul to have a look
before it can go in.

Wei.

Re: [PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Wei Liu]

On Tue, Aug 28, 2018 at 08:59:45AM -0600, Jan Beulich wrote:
> Checking them before the grant copy means nothing as to the validity of
> the incoming request. As we shouldn't make the new data live before
> having validated it, introduce a second instance of the mapping array.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  drivers/net/xen-netback/common.h    |    3 ++-
>  drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
>  drivers/net/xen-netback/interface.c |    3 ++-
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/common.h
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/common.h
> @@ -241,8 +241,9 @@ struct xenvif_hash_cache {
>  struct xenvif_hash {
>  	unsigned int alg;
>  	u32 flags;
> +	bool mapping_sel;
>  	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
> -	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
> +	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
>  	unsigned int size;
>  	struct xenvif_hash_cache cache;
>  };
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
> @@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
>  		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
>  
>  	vif->hash.size = size;
> -	memset(vif->hash.mapping, 0, sizeof(u32) * size);
> +	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
> +	       sizeof(u32) * size);
>  
>  	return XEN_NETIF_CTRL_STATUS_SUCCESS;
>  }
> @@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
>  u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
>  			    u32 off)
>  {
> -	u32 *mapping = vif->hash.mapping;
> +	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];

Can you rename this to inactive_mapping so the code can be followed more
easily?

The code looks correct to me, but I would like Paul to have a look
before it can go in.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Wei Liu]

On Tue, Aug 28, 2018 at 09:00:14AM -0600, Jan Beulich wrote:
> There's no guarantee that the mapping array doesn't cross a page
> boundary. Use a second grant copy operation if necessary.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH 3/3] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Wei Liu]

On Tue, Aug 28, 2018 at 09:00:14AM -0600, Jan Beulich wrote:
> There's no guarantee that the mapping array doesn't cross a page
> boundary. Use a second grant copy operation if necessary.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

RE: [PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 28 August 2018 16:00
> To: Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu <wei.liu2@citrix.com>
> Cc: davem@davemloft.net; xen-devel <xen-devel@lists.xenproject.org>;
> netdev@vger.kernel.org
> Subject: [PATCH 2/3] xen-netback: validate queue numbers in
> xenvif_set_hash_mapping()
> 
> Checking them before the grant copy means nothing as to the validity of
> the incoming request. As we shouldn't make the new data live before
> having validated it, introduce a second instance of the mapping array.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> 
> ---
>  drivers/net/xen-netback/common.h    |    3 ++-
>  drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
>  drivers/net/xen-netback/interface.c |    3 ++-
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/common.h
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/common.h
> @@ -241,8 +241,9 @@ struct xenvif_hash_cache {
>  struct xenvif_hash {
>  	unsigned int alg;
>  	u32 flags;
> +	bool mapping_sel;
>  	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
> -	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
> +	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
>  	unsigned int size;
>  	struct xenvif_hash_cache cache;
>  };
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/hash.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/hash.c
> @@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
>  		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> 
>  	vif->hash.size = size;
> -	memset(vif->hash.mapping, 0, sizeof(u32) * size);
> +	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
> +	       sizeof(u32) * size);
> 
>  	return XEN_NETIF_CTRL_STATUS_SUCCESS;
>  }
> @@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
>  u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
>  			    u32 off)
>  {
> -	u32 *mapping = vif->hash.mapping;
> +	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
>  	struct gnttab_copy copy_op = {
>  		.source.u.ref = gref,
>  		.source.domid = vif->domid,
> @@ -348,9 +349,8 @@ u32 xenvif_set_hash_mapping(struct xenvi
>  	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
>  	copy_op.dest.offset = xen_offset_in_page(mapping + off);
> 
> -	while (len-- != 0)
> -		if (mapping[off++] >= vif->num_queues)
> -			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> +	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
> +	       vif->hash.size * sizeof(*mapping));
> 
>  	if (copy_op.len != 0) {
>  		gnttab_batch_copy(&copy_op, 1);
> @@ -359,6 +359,12 @@ u32 xenvif_set_hash_mapping(struct xenvi
>  			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
>  	}
> 
> +	while (len-- != 0)
> +		if (mapping[off++] >= vif->num_queues)
> +			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> +
> +	vif->hash.mapping_sel = !vif->hash.mapping_sel;
> +
>  	return XEN_NETIF_CTRL_STATUS_SUCCESS;
>  }
> 
> @@ -410,6 +416,8 @@ void xenvif_dump_hash_info(struct xenvif
>  	}
> 
>  	if (vif->hash.size != 0) {
> +		const u32 *mapping = vif->hash.mapping[vif-
> >hash.mapping_sel];
> +
>  		seq_puts(m, "\nHash Mapping:\n");
> 
>  		for (i = 0; i < vif->hash.size; ) {
> @@ -422,7 +430,7 @@ void xenvif_dump_hash_info(struct xenvif
>  			seq_printf(m, "[%4u - %4u]: ", i, i + n - 1);
> 
>  			for (j = 0; j < n; j++, i++)
> -				seq_printf(m, "%4u ", vif->hash.mapping[i]);
> +				seq_printf(m, "%4u ", mapping[i]);
> 
>  			seq_puts(m, "\n");
>  		}
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/interface.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/interface.c
> @@ -162,7 +162,8 @@ static u16 xenvif_select_queue(struct ne
>  	if (size == 0)
>  		return skb_get_hash_raw(skb) % dev-
> >real_num_tx_queues;
> 
> -	return vif->hash.mapping[skb_get_hash_raw(skb) % size];
> +	return vif->hash.mapping[vif->hash.mapping_sel]
> +				[skb_get_hash_raw(skb) % size];
>  }
> 
>  static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
> 
> 
> 

Re: [PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 28 August 2018 16:00
> To: Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu <wei.liu2@citrix.com>
> Cc: davem@davemloft.net; xen-devel <xen-devel@lists.xenproject.org>;
> netdev@vger.kernel.org
> Subject: [PATCH 2/3] xen-netback: validate queue numbers in
> xenvif_set_hash_mapping()
> 
> Checking them before the grant copy means nothing as to the validity of
> the incoming request. As we shouldn't make the new data live before
> having validated it, introduce a second instance of the mapping array.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> 
> ---
>  drivers/net/xen-netback/common.h    |    3 ++-
>  drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
>  drivers/net/xen-netback/interface.c |    3 ++-
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/common.h
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/common.h
> @@ -241,8 +241,9 @@ struct xenvif_hash_cache {
>  struct xenvif_hash {
>  	unsigned int alg;
>  	u32 flags;
> +	bool mapping_sel;
>  	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
> -	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
> +	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
>  	unsigned int size;
>  	struct xenvif_hash_cache cache;
>  };
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/hash.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/hash.c
> @@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
>  		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> 
>  	vif->hash.size = size;
> -	memset(vif->hash.mapping, 0, sizeof(u32) * size);
> +	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
> +	       sizeof(u32) * size);
> 
>  	return XEN_NETIF_CTRL_STATUS_SUCCESS;
>  }
> @@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
>  u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
>  			    u32 off)
>  {
> -	u32 *mapping = vif->hash.mapping;
> +	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
>  	struct gnttab_copy copy_op = {
>  		.source.u.ref = gref,
>  		.source.domid = vif->domid,
> @@ -348,9 +349,8 @@ u32 xenvif_set_hash_mapping(struct xenvi
>  	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
>  	copy_op.dest.offset = xen_offset_in_page(mapping + off);
> 
> -	while (len-- != 0)
> -		if (mapping[off++] >= vif->num_queues)
> -			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> +	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
> +	       vif->hash.size * sizeof(*mapping));
> 
>  	if (copy_op.len != 0) {
>  		gnttab_batch_copy(&copy_op, 1);
> @@ -359,6 +359,12 @@ u32 xenvif_set_hash_mapping(struct xenvi
>  			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
>  	}
> 
> +	while (len-- != 0)
> +		if (mapping[off++] >= vif->num_queues)
> +			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> +
> +	vif->hash.mapping_sel = !vif->hash.mapping_sel;
> +
>  	return XEN_NETIF_CTRL_STATUS_SUCCESS;
>  }
> 
> @@ -410,6 +416,8 @@ void xenvif_dump_hash_info(struct xenvif
>  	}
> 
>  	if (vif->hash.size != 0) {
> +		const u32 *mapping = vif->hash.mapping[vif-
> >hash.mapping_sel];
> +
>  		seq_puts(m, "\nHash Mapping:\n");
> 
>  		for (i = 0; i < vif->hash.size; ) {
> @@ -422,7 +430,7 @@ void xenvif_dump_hash_info(struct xenvif
>  			seq_printf(m, "[%4u - %4u]: ", i, i + n - 1);
> 
>  			for (j = 0; j < n; j++, i++)
> -				seq_printf(m, "%4u ", vif->hash.mapping[i]);
> +				seq_printf(m, "%4u ", mapping[i]);
> 
>  			seq_puts(m, "\n");
>  		}
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/interface.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/interface.c
> @@ -162,7 +162,8 @@ static u16 xenvif_select_queue(struct ne
>  	if (size == 0)
>  		return skb_get_hash_raw(skb) % dev-
> >real_num_tx_queues;
> 
> -	return vif->hash.mapping[skb_get_hash_raw(skb) % size];
> +	return vif->hash.mapping[vif->hash.mapping_sel]
> +				[skb_get_hash_raw(skb) % size];
>  }
> 
>  static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
> 
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

RE: [PATCH 3/3] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 28 August 2018 16:00
> To: Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu <wei.liu2@citrix.com>
> Cc: davem@davemloft.net; xen-devel <xen-devel@lists.xenproject.org>;
> netdev@vger.kernel.org
> Subject: [PATCH 3/3] xen-netback: handle page straddling in
> xenvif_set_hash_mapping()
> 
> There's no guarantee that the mapping array doesn't cross a page
> boundary. Use a second grant copy operation if necessary.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Personally I think it would be cleaner to out-of-line the allocation of the mapping table and ensure it is page aligned but this works so...

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> 
> ---
>  drivers/net/xen-netback/hash.c |   25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/hash.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/hash.c
> @@ -334,28 +334,39 @@ u32 xenvif_set_hash_mapping(struct xenvi
>  			    u32 off)
>  {
>  	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
> -	struct gnttab_copy copy_op = {
> +	unsigned int nr = 1;
> +	struct gnttab_copy copy_op[2] = {{
>  		.source.u.ref = gref,
>  		.source.domid = vif->domid,
>  		.dest.domid = DOMID_SELF,
>  		.len = len * sizeof(*mapping),
>  		.flags = GNTCOPY_source_gref
> -	};
> +	}};
> 
>  	if ((off + len < off) || (off + len > vif->hash.size) ||
>  	    len > XEN_PAGE_SIZE / sizeof(*mapping))
>  		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> 
> -	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
> -	copy_op.dest.offset = xen_offset_in_page(mapping + off);
> +	copy_op[0].dest.u.gmfn = virt_to_gfn(mapping + off);
> +	copy_op[0].dest.offset = xen_offset_in_page(mapping + off);
> +	if (copy_op[0].dest.offset + copy_op[0].len > XEN_PAGE_SIZE) {
> +		copy_op[1] = copy_op[0];
> +		copy_op[1].source.offset = XEN_PAGE_SIZE -
> copy_op[0].dest.offset;
> +		copy_op[1].dest.u.gmfn = virt_to_gfn(mapping + off + len);
> +		copy_op[1].dest.offset = 0;
> +		copy_op[1].len = copy_op[0].len - copy_op[1].source.offset;
> +		copy_op[0].len = copy_op[1].source.offset;
> +		nr = 2;
> +	}
> 
>  	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
>  	       vif->hash.size * sizeof(*mapping));
> 
> -	if (copy_op.len != 0) {
> -		gnttab_batch_copy(&copy_op, 1);
> +	if (copy_op[0].len != 0) {
> +		gnttab_batch_copy(copy_op, nr);
> 
> -		if (copy_op.status != GNTST_okay)
> +		if (copy_op[0].status != GNTST_okay ||
> +		    copy_op[nr - 1].status != GNTST_okay)
>  			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
>  	}
> 
> 
> 
> 

Re: [PATCH 3/3] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 28 August 2018 16:00
> To: Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu <wei.liu2@citrix.com>
> Cc: davem@davemloft.net; xen-devel <xen-devel@lists.xenproject.org>;
> netdev@vger.kernel.org
> Subject: [PATCH 3/3] xen-netback: handle page straddling in
> xenvif_set_hash_mapping()
> 
> There's no guarantee that the mapping array doesn't cross a page
> boundary. Use a second grant copy operation if necessary.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Personally I think it would be cleaner to out-of-line the allocation of the mapping table and ensure it is page aligned but this works so...

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> 
> ---
>  drivers/net/xen-netback/hash.c |   25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> --- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-
> netback/hash.c
> +++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-
> netback/hash.c
> @@ -334,28 +334,39 @@ u32 xenvif_set_hash_mapping(struct xenvi
>  			    u32 off)
>  {
>  	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
> -	struct gnttab_copy copy_op = {
> +	unsigned int nr = 1;
> +	struct gnttab_copy copy_op[2] = {{
>  		.source.u.ref = gref,
>  		.source.domid = vif->domid,
>  		.dest.domid = DOMID_SELF,
>  		.len = len * sizeof(*mapping),
>  		.flags = GNTCOPY_source_gref
> -	};
> +	}};
> 
>  	if ((off + len < off) || (off + len > vif->hash.size) ||
>  	    len > XEN_PAGE_SIZE / sizeof(*mapping))
>  		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
> 
> -	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
> -	copy_op.dest.offset = xen_offset_in_page(mapping + off);
> +	copy_op[0].dest.u.gmfn = virt_to_gfn(mapping + off);
> +	copy_op[0].dest.offset = xen_offset_in_page(mapping + off);
> +	if (copy_op[0].dest.offset + copy_op[0].len > XEN_PAGE_SIZE) {
> +		copy_op[1] = copy_op[0];
> +		copy_op[1].source.offset = XEN_PAGE_SIZE -
> copy_op[0].dest.offset;
> +		copy_op[1].dest.u.gmfn = virt_to_gfn(mapping + off + len);
> +		copy_op[1].dest.offset = 0;
> +		copy_op[1].len = copy_op[0].len - copy_op[1].source.offset;
> +		copy_op[0].len = copy_op[1].source.offset;
> +		nr = 2;
> +	}
> 
>  	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
>  	       vif->hash.size * sizeof(*mapping));
> 
> -	if (copy_op.len != 0) {
> -		gnttab_batch_copy(&copy_op, 1);
> +	if (copy_op[0].len != 0) {
> +		gnttab_batch_copy(copy_op, nr);
> 
> -		if (copy_op.status != GNTST_okay)
> +		if (copy_op[0].status != GNTST_okay ||
> +		    copy_op[nr - 1].status != GNTST_okay)
>  			return
> XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
>  	}
> 
> 
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Jan Beulich]

>>> On 28.08.18 at 16:54,  wrote:
> First and foremost the fix for XSA-270. On top of that further changes
> which looked desirable to me while investigating that XSA.
> 
> 1: fix input validation in xenvif_set_hash_mapping()
> 2: validate queue numbers in xenvif_set_hash_mapping()
> 3: handle page straddling in xenvif_set_hash_mapping()
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

What is the way forward here? I've got R-b-s from Paul for all three
patches, and a minor change request on patch 2 from Wei. I'm not
really certain what to do in this case (hints appreciated), but could
at least the security fix (patch 1) be applied immediately?

Jan

Re: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Jan Beulich]

>>> On 28.08.18 at 16:54,  wrote:
> First and foremost the fix for XSA-270. On top of that further changes
> which looked desirable to me while investigating that XSA.
> 
> 1: fix input validation in xenvif_set_hash_mapping()
> 2: validate queue numbers in xenvif_set_hash_mapping()
> 3: handle page straddling in xenvif_set_hash_mapping()
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

What is the way forward here? I've got R-b-s from Paul for all three
patches, and a minor change request on patch 2 from Wei. I'm not
really certain what to do in this case (hints appreciated), but could
at least the security fix (patch 1) be applied immediately?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Wei Liu]

On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
> >>> On 28.08.18 at 16:54,  wrote:
> > First and foremost the fix for XSA-270. On top of that further changes
> > which looked desirable to me while investigating that XSA.
> > 
> > 1: fix input validation in xenvif_set_hash_mapping()
> > 2: validate queue numbers in xenvif_set_hash_mapping()
> > 3: handle page straddling in xenvif_set_hash_mapping()
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> What is the way forward here? I've got R-b-s from Paul for all three
> patches, and a minor change request on patch 2 from Wei. I'm not
> really certain what to do in this case (hints appreciated), but could
> at least the security fix (patch 1) be applied immediately?

If you happen to resend, please make the adjustment; otherwise I'm fine
with the patches as they are. I don't want to block useful things on
cosmetic issues.

Wei.

Re: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Wei Liu]

On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
> >>> On 28.08.18 at 16:54,  wrote:
> > First and foremost the fix for XSA-270. On top of that further changes
> > which looked desirable to me while investigating that XSA.
> > 
> > 1: fix input validation in xenvif_set_hash_mapping()
> > 2: validate queue numbers in xenvif_set_hash_mapping()
> > 3: handle page straddling in xenvif_set_hash_mapping()
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> What is the way forward here? I've got R-b-s from Paul for all three
> patches, and a minor change request on patch 2 from Wei. I'm not
> really certain what to do in this case (hints appreciated), but could
> at least the security fix (patch 1) be applied immediately?

If you happen to resend, please make the adjustment; otherwise I'm fine
with the patches as they are. I don't want to block useful things on
cosmetic issues.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Ping: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Jan Beulich]

>>> On 11.09.18 at 12:16, <wei.liu2@citrix.com> wrote:
> On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
>> >>> On 28.08.18 at 16:54,  wrote:
>> > First and foremost the fix for XSA-270. On top of that further changes
>> > which looked desirable to me while investigating that XSA.
>> > 
>> > 1: fix input validation in xenvif_set_hash_mapping()
>> > 2: validate queue numbers in xenvif_set_hash_mapping()
>> > 3: handle page straddling in xenvif_set_hash_mapping()
>> > 
>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> What is the way forward here? I've got R-b-s from Paul for all three
>> patches, and a minor change request on patch 2 from Wei. I'm not
>> really certain what to do in this case (hints appreciated), but could
>> at least the security fix (patch 1) be applied immediately?
> 
> If you happen to resend, please make the adjustment; otherwise I'm fine
> with the patches as they are. I don't want to block useful things on
> cosmetic issues.

Dave? I notice none of the patches is in 4.19-rc5, not even the security
fix, the advisory for which had gone public over a month ago.

Jan

Ping: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Jan Beulich]

>>> On 11.09.18 at 12:16, <wei.liu2@citrix.com> wrote:
> On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
>> >>> On 28.08.18 at 16:54,  wrote:
>> > First and foremost the fix for XSA-270. On top of that further changes
>> > which looked desirable to me while investigating that XSA.
>> > 
>> > 1: fix input validation in xenvif_set_hash_mapping()
>> > 2: validate queue numbers in xenvif_set_hash_mapping()
>> > 3: handle page straddling in xenvif_set_hash_mapping()
>> > 
>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> What is the way forward here? I've got R-b-s from Paul for all three
>> patches, and a minor change request on patch 2 from Wei. I'm not
>> really certain what to do in this case (hints appreciated), but could
>> at least the security fix (patch 1) be applied immediately?
> 
> If you happen to resend, please make the adjustment; otherwise I'm fine
> with the patches as they are. I don't want to block useful things on
> cosmetic issues.

Dave? I notice none of the patches is in 4.19-rc5, not even the security
fix, the advisory for which had gone public over a month ago.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Ping: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[David Miller]

From: "Jan Beulich" <JBeulich@suse.com>
Date: Mon, 24 Sep 2018 01:43:50 -0600

>>>> On 11.09.18 at 12:16, <wei.liu2@citrix.com> wrote:
>> On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
>>> >>> On 28.08.18 at 16:54,  wrote:
>>> > First and foremost the fix for XSA-270. On top of that further changes
>>> > which looked desirable to me while investigating that XSA.
>>> > 
>>> > 1: fix input validation in xenvif_set_hash_mapping()
>>> > 2: validate queue numbers in xenvif_set_hash_mapping()
>>> > 3: handle page straddling in xenvif_set_hash_mapping()
>>> > 
>>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> 
>>> What is the way forward here? I've got R-b-s from Paul for all three
>>> patches, and a minor change request on patch 2 from Wei. I'm not
>>> really certain what to do in this case (hints appreciated), but could
>>> at least the security fix (patch 1) be applied immediately?
>> 
>> If you happen to resend, please make the adjustment; otherwise I'm fine
>> with the patches as they are. I don't want to block useful things on
>> cosmetic issues.
> 
> Dave? I notice none of the patches is in 4.19-rc5, not even the security
> fix, the advisory for which had gone public over a month ago.

If it's not in my patchwork queue, you have to resend the series and
make it clear that it should be applied to the networking tree by
putting "[PATCH net N/M]" in the Subject lines.

Thank you.

Re: Ping: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[David Miller]

From: "Jan Beulich" <JBeulich@suse.com>
Date: Mon, 24 Sep 2018 01:43:50 -0600

>>>> On 11.09.18 at 12:16, <wei.liu2@citrix.com> wrote:
>> On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
>>> >>> On 28.08.18 at 16:54,  wrote:
>>> > First and foremost the fix for XSA-270. On top of that further changes
>>> > which looked desirable to me while investigating that XSA.
>>> > 
>>> > 1: fix input validation in xenvif_set_hash_mapping()
>>> > 2: validate queue numbers in xenvif_set_hash_mapping()
>>> > 3: handle page straddling in xenvif_set_hash_mapping()
>>> > 
>>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> 
>>> What is the way forward here? I've got R-b-s from Paul for all three
>>> patches, and a minor change request on patch 2 from Wei. I'm not
>>> really certain what to do in this case (hints appreciated), but could
>>> at least the security fix (patch 1) be applied immediately?
>> 
>> If you happen to resend, please make the adjustment; otherwise I'm fine
>> with the patches as they are. I don't want to block useful things on
>> cosmetic issues.
> 
> Dave? I notice none of the patches is in 4.19-rc5, not even the security
> fix, the advisory for which had gone public over a month ago.

If it's not in my patchwork queue, you have to resend the series and
make it clear that it should be applied to the networking tree by
putting "[PATCH net N/M]" in the Subject lines.

Thank you.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH net 0/3 RESEND] xen-netback: hash mapping handling adjustments

[Jan Beulich]

(re-send just to satisfy the apparent need for "net" inside the square brackets)

First and foremost the fix for XSA-270. On top of that further changes
which looked desirable to me while investigating that XSA.

1: fix input validation in xenvif_set_hash_mapping()
2: validate queue numbers in xenvif_set_hash_mapping()
3: handle page straddling in xenvif_set_hash_mapping()

Signed-off-by: Jan Beulich <jbeulich@suse.com>

[PATCH net 0/3 RESEND] xen-netback: hash mapping handling adjustments

[Jan Beulich]

(re-send just to satisfy the apparent need for "net" inside the square brackets)

First and foremost the fix for XSA-270. On top of that further changes
which looked desirable to me while investigating that XSA.

1: fix input validation in xenvif_set_hash_mapping()
2: validate queue numbers in xenvif_set_hash_mapping()
3: handle page straddling in xenvif_set_hash_mapping()

Signed-off-by: Jan Beulich <jbeulich@suse.com>




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH net 1/3 RESEND] xen-netback: fix input validation in xenvif_set_hash_mapping()

[Jan Beulich]

Both len and off are frontend specified values, so we need to make
sure there's no overflow when adding the two for the bounds check. We
also want to avoid undefined behavior and hence use off to index into
->hash.mapping[] only after bounds checking. This at the same time
allows to take care of not applying off twice for the bounds checking
against vif->num_queues.

It is also insufficient to bounds check copy_op.len, as this is len
truncated to 16 bits.

This is XSA-270 / CVE-2018-15471.

Reported-by: Felix Wilhelm <fwilhelm@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Tested-by: Paul Durrant <paul.durrant@citrix.com>
Cc: stable@vger.kernel.org [4.7 onwards]
---
 drivers/net/xen-netback/hash.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -332,20 +332,22 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = &vif->hash.mapping[off];
+	u32 *mapping = vif->hash.mapping;
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
-		.dest.u.gmfn = virt_to_gfn(mapping),
 		.dest.domid = DOMID_SELF,
-		.dest.offset = xen_offset_in_page(mapping),
-		.len = len * sizeof(u32),
+		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
 	};
 
-	if ((off + len > vif->hash.size) || copy_op.len > XEN_PAGE_SIZE)
+	if ((off + len < off) || (off + len > vif->hash.size) ||
+	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
+	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+
 	while (len-- != 0)
 		if (mapping[off++] >= vif->num_queues)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;

[PATCH net 1/3 RESEND] xen-netback: fix input validation in xenvif_set_hash_mapping()

[Jan Beulich]

Both len and off are frontend specified values, so we need to make
sure there's no overflow when adding the two for the bounds check. We
also want to avoid undefined behavior and hence use off to index into
->hash.mapping[] only after bounds checking. This at the same time
allows to take care of not applying off twice for the bounds checking
against vif->num_queues.

It is also insufficient to bounds check copy_op.len, as this is len
truncated to 16 bits.

This is XSA-270 / CVE-2018-15471.

Reported-by: Felix Wilhelm <fwilhelm@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Tested-by: Paul Durrant <paul.durrant@citrix.com>
Cc: stable@vger.kernel.org [4.7 onwards]
---
 drivers/net/xen-netback/hash.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -332,20 +332,22 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = &vif->hash.mapping[off];
+	u32 *mapping = vif->hash.mapping;
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
-		.dest.u.gmfn = virt_to_gfn(mapping),
 		.dest.domid = DOMID_SELF,
-		.dest.offset = xen_offset_in_page(mapping),
-		.len = len * sizeof(u32),
+		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
 	};
 
-	if ((off + len > vif->hash.size) || copy_op.len > XEN_PAGE_SIZE)
+	if ((off + len < off) || (off + len > vif->hash.size) ||
+	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
+	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+
 	while (len-- != 0)
 		if (mapping[off++] >= vif->num_queues)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH net 2/3 RESEND] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Jan Beulich]

Checking them before the grant copy means nothing as to the validity of
the incoming request. As we shouldn't make the new data live before
having validated it, introduce a second instance of the mapping array.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
 drivers/net/xen-netback/common.h    |    3 ++-
 drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
 drivers/net/xen-netback/interface.c |    3 ++-
 3 files changed, 18 insertions(+), 8 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/common.h
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/common.h
@@ -241,8 +241,9 @@ struct xenvif_hash_cache {
 struct xenvif_hash {
 	unsigned int alg;
 	u32 flags;
+	bool mapping_sel;
 	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
-	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
+	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
 	unsigned int size;
 	struct xenvif_hash_cache cache;
 };
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
 	vif->hash.size = size;
-	memset(vif->hash.mapping, 0, sizeof(u32) * size);
+	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
+	       sizeof(u32) * size);
 
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
@@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = vif->hash.mapping;
+	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
@@ -348,9 +349,8 @@ u32 xenvif_set_hash_mapping(struct xenvi
 	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
 	copy_op.dest.offset = xen_offset_in_page(mapping + off);
 
-	while (len-- != 0)
-		if (mapping[off++] >= vif->num_queues)
-			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
+	       vif->hash.size * sizeof(*mapping));
 
 	if (copy_op.len != 0) {
 		gnttab_batch_copy(&copy_op, 1);
@@ -359,6 +359,12 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 
+	while (len-- != 0)
+		if (mapping[off++] >= vif->num_queues)
+			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+
+	vif->hash.mapping_sel = !vif->hash.mapping_sel;
+
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
 
@@ -410,6 +416,8 @@ void xenvif_dump_hash_info(struct xenvif
 	}
 
 	if (vif->hash.size != 0) {
+		const u32 *mapping = vif->hash.mapping[vif->hash.mapping_sel];
+
 		seq_puts(m, "\nHash Mapping:\n");
 
 		for (i = 0; i < vif->hash.size; ) {
@@ -422,7 +430,7 @@ void xenvif_dump_hash_info(struct xenvif
 			seq_printf(m, "[%4u - %4u]: ", i, i + n - 1);
 
 			for (j = 0; j < n; j++, i++)
-				seq_printf(m, "%4u ", vif->hash.mapping[i]);
+				seq_printf(m, "%4u ", mapping[i]);
 
 			seq_puts(m, "\n");
 		}
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/interface.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/interface.c
@@ -162,7 +162,8 @@ static u16 xenvif_select_queue(struct ne
 	if (size == 0)
 		return skb_get_hash_raw(skb) % dev->real_num_tx_queues;
 
-	return vif->hash.mapping[skb_get_hash_raw(skb) % size];
+	return vif->hash.mapping[vif->hash.mapping_sel]
+				[skb_get_hash_raw(skb) % size];
 }
 
 static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)

[PATCH net 2/3 RESEND] xen-netback: validate queue numbers in xenvif_set_hash_mapping()

[Jan Beulich]

Checking them before the grant copy means nothing as to the validity of
the incoming request. As we shouldn't make the new data live before
having validated it, introduce a second instance of the mapping array.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
 drivers/net/xen-netback/common.h    |    3 ++-
 drivers/net/xen-netback/hash.c      |   20 ++++++++++++++------
 drivers/net/xen-netback/interface.c |    3 ++-
 3 files changed, 18 insertions(+), 8 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/common.h
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/common.h
@@ -241,8 +241,9 @@ struct xenvif_hash_cache {
 struct xenvif_hash {
 	unsigned int alg;
 	u32 flags;
+	bool mapping_sel;
 	u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
-	u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
+	u32 mapping[2][XEN_NETBK_MAX_HASH_MAPPING_SIZE];
 	unsigned int size;
 	struct xenvif_hash_cache cache;
 };
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -324,7 +324,8 @@ u32 xenvif_set_hash_mapping_size(struct
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
 	vif->hash.size = size;
-	memset(vif->hash.mapping, 0, sizeof(u32) * size);
+	memset(vif->hash.mapping[vif->hash.mapping_sel], 0,
+	       sizeof(u32) * size);
 
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
@@ -332,7 +333,7 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = vif->hash.mapping;
+	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
@@ -348,9 +349,8 @@ u32 xenvif_set_hash_mapping(struct xenvi
 	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
 	copy_op.dest.offset = xen_offset_in_page(mapping + off);
 
-	while (len-- != 0)
-		if (mapping[off++] >= vif->num_queues)
-			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
+	       vif->hash.size * sizeof(*mapping));
 
 	if (copy_op.len != 0) {
 		gnttab_batch_copy(&copy_op, 1);
@@ -359,6 +359,12 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 
+	while (len-- != 0)
+		if (mapping[off++] >= vif->num_queues)
+			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
+
+	vif->hash.mapping_sel = !vif->hash.mapping_sel;
+
 	return XEN_NETIF_CTRL_STATUS_SUCCESS;
 }
 
@@ -410,6 +416,8 @@ void xenvif_dump_hash_info(struct xenvif
 	}
 
 	if (vif->hash.size != 0) {
+		const u32 *mapping = vif->hash.mapping[vif->hash.mapping_sel];
+
 		seq_puts(m, "\nHash Mapping:\n");
 
 		for (i = 0; i < vif->hash.size; ) {
@@ -422,7 +430,7 @@ void xenvif_dump_hash_info(struct xenvif
 			seq_printf(m, "[%4u - %4u]: ", i, i + n - 1);
 
 			for (j = 0; j < n; j++, i++)
-				seq_printf(m, "%4u ", vif->hash.mapping[i]);
+				seq_printf(m, "%4u ", mapping[i]);
 
 			seq_puts(m, "\n");
 		}
--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/interface.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/interface.c
@@ -162,7 +162,8 @@ static u16 xenvif_select_queue(struct ne
 	if (size == 0)
 		return skb_get_hash_raw(skb) % dev->real_num_tx_queues;
 
-	return vif->hash.mapping[skb_get_hash_raw(skb) % size];
+	return vif->hash.mapping[vif->hash.mapping_sel]
+				[skb_get_hash_raw(skb) % size];
 }
 
 static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH net 3/3 RESEND] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Jan Beulich]

There's no guarantee that the mapping array doesn't cross a page
boundary. Use a second grant copy operation if necessary.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
 drivers/net/xen-netback/hash.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -334,28 +334,39 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			    u32 off)
 {
 	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
-	struct gnttab_copy copy_op = {
+	unsigned int nr = 1;
+	struct gnttab_copy copy_op[2] = {{
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
 		.dest.domid = DOMID_SELF,
 		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
-	};
+	}};
 
 	if ((off + len < off) || (off + len > vif->hash.size) ||
 	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
-	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
-	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+	copy_op[0].dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op[0].dest.offset = xen_offset_in_page(mapping + off);
+	if (copy_op[0].dest.offset + copy_op[0].len > XEN_PAGE_SIZE) {
+		copy_op[1] = copy_op[0];
+		copy_op[1].source.offset = XEN_PAGE_SIZE - copy_op[0].dest.offset;
+		copy_op[1].dest.u.gmfn = virt_to_gfn(mapping + off + len);
+		copy_op[1].dest.offset = 0;
+		copy_op[1].len = copy_op[0].len - copy_op[1].source.offset;
+		copy_op[0].len = copy_op[1].source.offset;
+		nr = 2;
+	}
 
 	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
 	       vif->hash.size * sizeof(*mapping));
 
-	if (copy_op.len != 0) {
-		gnttab_batch_copy(&copy_op, 1);
+	if (copy_op[0].len != 0) {
+		gnttab_batch_copy(copy_op, nr);
 
-		if (copy_op.status != GNTST_okay)
+		if (copy_op[0].status != GNTST_okay ||
+		    copy_op[nr - 1].status != GNTST_okay)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 

[PATCH net 3/3 RESEND] xen-netback: handle page straddling in xenvif_set_hash_mapping()

[Jan Beulich]

There's no guarantee that the mapping array doesn't cross a page
boundary. Use a second grant copy operation if necessary.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
 drivers/net/xen-netback/hash.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

--- 4.19-rc1-xen-netback-set-hash-mapping.orig/drivers/net/xen-netback/hash.c
+++ 4.19-rc1-xen-netback-set-hash-mapping/drivers/net/xen-netback/hash.c
@@ -334,28 +334,39 @@ u32 xenvif_set_hash_mapping(struct xenvi
 			    u32 off)
 {
 	u32 *mapping = vif->hash.mapping[!vif->hash.mapping_sel];
-	struct gnttab_copy copy_op = {
+	unsigned int nr = 1;
+	struct gnttab_copy copy_op[2] = {{
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
 		.dest.domid = DOMID_SELF,
 		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
-	};
+	}};
 
 	if ((off + len < off) || (off + len > vif->hash.size) ||
 	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
-	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
-	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+	copy_op[0].dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op[0].dest.offset = xen_offset_in_page(mapping + off);
+	if (copy_op[0].dest.offset + copy_op[0].len > XEN_PAGE_SIZE) {
+		copy_op[1] = copy_op[0];
+		copy_op[1].source.offset = XEN_PAGE_SIZE - copy_op[0].dest.offset;
+		copy_op[1].dest.u.gmfn = virt_to_gfn(mapping + off + len);
+		copy_op[1].dest.offset = 0;
+		copy_op[1].len = copy_op[0].len - copy_op[1].source.offset;
+		copy_op[0].len = copy_op[1].source.offset;
+		nr = 2;
+	}
 
 	memcpy(mapping, vif->hash.mapping[vif->hash.mapping_sel],
 	       vif->hash.size * sizeof(*mapping));
 
-	if (copy_op.len != 0) {
-		gnttab_batch_copy(&copy_op, 1);
+	if (copy_op[0].len != 0) {
+		gnttab_batch_copy(copy_op, nr);
 
-		if (copy_op.status != GNTST_okay)
+		if (copy_op[0].status != GNTST_okay ||
+		    copy_op[nr - 1].status != GNTST_okay)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 	}
 





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Ping: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Jan Beulich]

>>> On 24.09.18 at 18:49, <davem@davemloft.net> wrote:
> From: "Jan Beulich" <JBeulich@suse.com>
> Date: Mon, 24 Sep 2018 01:43:50 -0600
> 
>> Dave? I notice none of the patches is in 4.19-rc5, not even the security
>> fix, the advisory for which had gone public over a month ago.
> 
> If it's not in my patchwork queue, you have to resend the series and
> make it clear that it should be applied to the networking tree by
> putting "[PATCH net N/M]" in the Subject lines.

I'm sure this requirement is documented somewhere, but some searching
in Documentation/process/ did not turn up anything. For the occasional
contributor, such extra tag addition requirements are pretty unhelpful.

Anyway - I've done the resend, hopefully now meeting the requirements
to avoid further delays.

Jan

Re: Ping: [PATCH 0/3] xen-netback: hash mapping hanling adjustments

[Jan Beulich]

>>> On 24.09.18 at 18:49, <davem@davemloft.net> wrote:
> From: "Jan Beulich" <JBeulich@suse.com>
> Date: Mon, 24 Sep 2018 01:43:50 -0600
> 
>> Dave? I notice none of the patches is in 4.19-rc5, not even the security
>> fix, the advisory for which had gone public over a month ago.
> 
> If it's not in my patchwork queue, you have to resend the series and
> make it clear that it should be applied to the networking tree by
> putting "[PATCH net N/M]" in the Subject lines.

I'm sure this requirement is documented somewhere, but some searching
in Documentation/process/ did not turn up anything. For the occasional
contributor, such extra tag addition requirements are pretty unhelpful.

Anyway - I've done the resend, hopefully now meeting the requirements
to avoid further delays.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH net 0/3 RESEND] xen-netback: hash mapping handling adjustments

[David Miller]

From: "Jan Beulich" <JBeulich@suse.com>
Date: Tue, 25 Sep 2018 02:11:33 -0600

> First and foremost the fix for XSA-270. On top of that further changes
> which looked desirable to me while investigating that XSA.
> 
> 1: fix input validation in xenvif_set_hash_mapping()
> 2: validate queue numbers in xenvif_set_hash_mapping()
> 3: handle page straddling in xenvif_set_hash_mapping()
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Series applied and queued up for -stable.

Thanks.

Re: [PATCH net 0/3 RESEND] xen-netback: hash mapping handling adjustments

[David Miller]

From: "Jan Beulich" <JBeulich@suse.com>
Date: Tue, 25 Sep 2018 02:11:33 -0600

> First and foremost the fix for XSA-270. On top of that further changes
> which looked desirable to me while investigating that XSA.
> 
> 1: fix input validation in xenvif_set_hash_mapping()
> 2: validate queue numbers in xenvif_set_hash_mapping()
> 3: handle page straddling in xenvif_set_hash_mapping()
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Series applied and queued up for -stable.

Thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel