|
 |
fc841cf |
From: Jan Beulich <jbeulich@suse.com>
|
|
|
fc841cf |
Subject: xen-netback: fix input validation in xenvif_set_hash_mapping()
|
|
|
fc841cf |
|
|
|
fc841cf |
Both len and off are frontend specified values, so we need to make
|
|
|
fc841cf |
sure there's no overflow when adding the two for the bounds check. We
|
|
|
fc841cf |
also want to avoid undefined behavior and hence use off to index into
|
|
|
fc841cf |
->hash.mapping[] only after bounds checking. This at the same time
|
|
|
fc841cf |
allows to take care of not applying off twice for the bounds checking
|
|
|
fc841cf |
against vif->num_queues.
|
|
|
fc841cf |
|
|
|
fc841cf |
It is also insufficient to bounds check copy_op.len, as this is len
|
|
|
fc841cf |
truncated to 16 bits.
|
|
|
fc841cf |
|
|
|
fc841cf |
This is XSA-270.
|
|
|
fc841cf |
|
|
|
fc841cf |
Reported-by: Felix Wilhelm <fwilhelm@google.com>
|
|
|
fc841cf |
Signed-off-by: Jan Beulich <jbeulich@suse.com>
|
|
|
fc841cf |
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
|
|
|
fc841cf |
Tested-by: Paul Durrant <paul.durrant@citrix.com>
|
|
|
fc841cf |
---
|
|
|
fc841cf |
The bounds checking against vif->num_queues also occurs too early afaict
|
|
|
fc841cf |
(it should be done after the grant copy). I have patches ready as public
|
|
|
fc841cf |
follow-ups for both this and the (at least latent) issue of the mapping
|
|
|
fc841cf |
array crossing a page boundary.
|
|
|
fc841cf |
|
|
|
fc841cf |
--- a/drivers/net/xen-netback/hash.c
|
|
|
fc841cf |
+++ b/drivers/net/xen-netback/hash.c
|
|
|
fc841cf |
@@ -332,20 +332,22 @@ u32 xenvif_set_hash_mapping_size(struct
|
|
|
fc841cf |
u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
|
|
|
fc841cf |
u32 off)
|
|
|
fc841cf |
{
|
|
|
fc841cf |
- u32 *mapping = &vif->hash.mapping[off];
|
|
|
fc841cf |
+ u32 *mapping = vif->hash.mapping;
|
|
|
fc841cf |
struct gnttab_copy copy_op = {
|
|
|
fc841cf |
.source.u.ref = gref,
|
|
|
fc841cf |
.source.domid = vif->domid,
|
|
|
fc841cf |
- .dest.u.gmfn = virt_to_gfn(mapping),
|
|
|
fc841cf |
.dest.domid = DOMID_SELF,
|
|
|
fc841cf |
- .dest.offset = xen_offset_in_page(mapping),
|
|
|
fc841cf |
- .len = len * sizeof(u32),
|
|
|
fc841cf |
+ .len = len * sizeof(*mapping),
|
|
|
fc841cf |
.flags = GNTCOPY_source_gref
|
|
|
fc841cf |
};
|
|
|
fc841cf |
|
|
|
fc841cf |
- if ((off + len > vif->hash.size) || copy_op.len > XEN_PAGE_SIZE)
|
|
|
fc841cf |
+ if ((off + len < off) || (off + len > vif->hash.size) ||
|
|
|
fc841cf |
+ len > XEN_PAGE_SIZE / sizeof(*mapping))
|
|
|
fc841cf |
return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
|
|
|
fc841cf |
|
|
|
fc841cf |
+ copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
|
|
|
fc841cf |
+ copy_op.dest.offset = xen_offset_in_page(mapping + off);
|
|
|
fc841cf |
+
|
|
|
fc841cf |
while (len-- != 0)
|
|
|
fc841cf |
if (mapping[off++] >= vif->num_queues)
|
|
|
fc841cf |
return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
|