From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Durrant Subject: RE: [PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash_mapping() Date: Mon, 3 Sep 2018 09:23:53 +0000 Message-ID: <3127beaea7fc4ec68d5bb1ee48c4412f@AMSPEX02CL03.citrite.net> References: <5B85622302000078001E2A35@prv1-mh.provo.novell.com> <5B85636102000078001E2A4D@prv1-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: "davem@davemloft.net" , xen-devel , "netdev@vger.kernel.org" To: 'Jan Beulich' , Wei Liu Return-path: Received: from smtp.ctxuk.citrix.com ([185.25.65.24]:4141 "EHLO SMTP.EU.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726022AbeICNnM (ORCPT ); Mon, 3 Sep 2018 09:43:12 -0400 In-Reply-To: <5B85636102000078001E2A4D@prv1-mh.provo.novell.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 28 August 2018 16:00 > To: Paul Durrant ; Wei Liu > Cc: davem@davemloft.net; xen-devel ; > 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 Reviewed-by: Paul Durrant > > --- > 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(©_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) > > >