ACK: [SRU][Bionic][Cosmic][PATCH 1/1] xen-netback: fix input validation in xenvif_set_hash_mapping()

[Stefan Bader]

On 19.10.18 11:46, Kleber Sacilotto de Souza wrote:
> From: Jan Beulich <JBeulich at suse.com>
> 
> 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 at google.com>
> Signed-off-by: Jan Beulich <jbeulich at suse.com>
> Reviewed-by: Paul Durrant <paul.durrant at citrix.com>
> Tested-by: Paul Durrant <paul.durrant at citrix.com>
> Cc: stable at vger.kernel.org [4.7 onwards]
> Signed-off-by: David S. Miller <davem at davemloft.net>
> 
> CVE-2018-15471
> (cherry picked from commit 780e83c259fc33e8959fed8dfdad17e378d72b62)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  drivers/net/xen-netback/hash.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/hash.c b/drivers/net/xen-netback/hash.c
> index 3c4c58b9fe76..3b6fb5b3bdb2 100644
> --- 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 xenvif *vif, u32 size)
>  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;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20181019/41b45964/attachment.sig>