netdev - Re: [PATCH 2/3] xen-netback: validate queue numbers in xenvif_set_hash

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180829082508.dczaww7fowq2c3z3@citrix.com>
Date:   Wed, 29 Aug 2018 09:25:08 +0100
From:   Wei Liu <wei.liu2@...rix.com>
To:     Jan Beulich <JBeulich@...e.com>
CC:     Paul Durrant <paul.durrant@...rix.com>,
        Wei Liu <wei.liu2@...rix.com>, <davem@...emloft.net>,
        xen-devel <xen-devel@...ts.xenproject.org>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH 2/3] xen-netback: validate queue numbers in
 xenvif_set_hash_mapping()

On Tue, Aug 28, 2018 at 08:59:45AM -0600, Jan Beulich wrote:
> 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@...e.com>
> 
> ---
>  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];

Can you rename this to inactive_mapping so the code can be followed more
easily?

The code looks correct to me, but I would like Paul to have a look
before it can go in.

Wei.