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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 28 August 2018 16:00
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>;
> netdev@xxxxxxxxxxxxxxx
> 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@xxxxxxxx>

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

> 
> ---
>  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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel