56 lines
2.1 KiB
Diff
56 lines
2.1 KiB
Diff
From: Jan Beulich <jbeulich@suse.com>
|
|
Subject: xen-netback: fix input validation in xenvif_set_hash_mapping()
|
|
|
|
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.
|
|
|
|
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>
|
|
---
|
|
The bounds checking against vif->num_queues also occurs too early afaict
|
|
(it should be done after the grant copy). I have patches ready as public
|
|
follow-ups for both this and the (at least latent) issue of the mapping
|
|
array crossing a page boundary.
|
|
|
|
--- a/drivers/net/xen-netback/hash.c
|
|
+++ b/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;
|