netdev - Re: [PATCH V2 net-next] net/mlx5: Add MLX5_ARRAY_SET64 to fix BUILD_BUG

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: <20161012085022.GR9282@leon.nu>
Date:   Wed, 12 Oct 2016 11:50:22 +0300
From:   Leon Romanovsky <leonro@...lanox.com>
To:     "David S. Miller" <davem@...emloft.net>,
        Doug Ledford <dledford@...hat.com>
Cc:     Saeed Mahameed <saeedm@...lanox.com>, netdev@...r.kernel.org,
        Tom Herbert <tom@...bertland.com>,
        David Laight <David.Laight@...lab.com>, kernel-team@...com,
        RDMA mailing list <linux-rdma@...r.kernel.org>,
        Steve Wise <swise@...ngridcomputing.com>
Subject: Re: [PATCH V2 net-next] net/mlx5: Add MLX5_ARRAY_SET64 to fix
 BUILD_BUG_ON

On Wed, Oct 12, 2016 at 04:57:10AM +0300, Saeed Mahameed wrote:
> From: Tom Herbert <tom@...bertland.com>
>
> I am hitting this in mlx5:
>
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function
> reclaim_pages_cmd.clone.0:
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:346: error: call
> to __compiletime_assert_346 declared with attribute error:
> BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_out, pas[i]) % 64
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function give_pages:
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:291: error: call
> to __compiletime_assert_291 declared with attribute error:
> BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_in, pas[i]) % 64
>
> Problem is that this is doing a BUILD_BUG_ON on a non-constant
> expression because of trying to take offset of pas[i] in the
> structure.
>
> Fix is to create MLX5_ARRAY_SET64 that takes an additional argument
> that is the field index to separate between BUILD_BUG_ON on the array
> constant field and the indexed field to assign the value to.
> There are two callers of MLX5_SET64 that are trying to get a variable
> offset, change those to call MLX5_ARRAY_SET64 passing 'pas' and 'i'
> as the arguments to use in the offset check and the indexed value
> assignment.
>
> Fixes: a533ed5e179cd ("net/mlx5: Pages management commands via mlx5 ifc")
> Signed-off-by: Tom Herbert <tom@...bertland.com>
> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
> ---
>
> Hi Dave,
>
> I hope this version of this patch will make it to -rc1, I made
> some changes to the original version Tom submitted.  Following David Laight
> suggestion to separate the array index from the constant array field to have
> a more natural API.
>
> Thanks,
> Saeed.
>
>
>  drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c |  4 ++--
>  include/linux/mlx5/device.h                         | 13 +++++++++++--
>  2 files changed, 13 insertions(+), 4 deletions(-)

Hi Dave and Doug,

Any chances that this fix will be forwarded to Linus in merge window?

I don't know if you (Dave) had in mind to send additional pull request to
Linus in this merge window, but Doug is definitely planning, and it will
be awesome if it includes this fix in it.

Thanks

>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> index d458515..cc4fd61 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> @@ -287,7 +287,7 @@ retry:
>
>  			goto retry;
>  		}
> -		MLX5_SET64(manage_pages_in, in, pas[i], addr);
> +		MLX5_ARRAY_SET64(manage_pages_in, in, pas, i, addr);
>  	}
>
>  	MLX5_SET(manage_pages_in, in, opcode, MLX5_CMD_OP_MANAGE_PAGES);
> @@ -344,7 +344,7 @@ static int reclaim_pages_cmd(struct mlx5_core_dev *dev,
>  		if (fwp->func_id != func_id)
>  			continue;
>
> -		MLX5_SET64(manage_pages_out, out, pas[i], fwp->addr);
> +		MLX5_ARRAY_SET64(manage_pages_out, out, pas, i, fwp->addr);
>  		i++;
>  	}
>
> diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
> index 77c1417..5827614 100644
> --- a/include/linux/mlx5/device.h
> +++ b/include/linux/mlx5/device.h
> @@ -92,12 +92,21 @@ __mlx5_mask(typ, fld))
>  	___t; \
>  })
>
> -#define MLX5_SET64(typ, p, fld, v) do { \
> +#define __MLX5_SET64(typ, p, fld, v) do { \
>  	BUILD_BUG_ON(__mlx5_bit_sz(typ, fld) != 64); \
> -	BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
>  	*((__be64 *)(p) + __mlx5_64_off(typ, fld)) = cpu_to_be64(v); \
>  } while (0)
>
> +#define MLX5_SET64(typ, p, fld, v) do { \
> +	BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
> +	__MLX5_SET64(typ, p, fld, v); \
> +} while (0)
> +
> +#define MLX5_ARRAY_SET64(typ, p, fld, idx, v) do { \
> +	BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
> +	__MLX5_SET64(typ, p, fld[idx], v); \
> +} while (0)
> +
>  #define MLX5_GET64(typ, p, fld) be64_to_cpu(*((__be64 *)(p) + __mlx5_64_off(typ, fld)))
>
>  #define MLX5_GET64_PR(typ, p, fld) ({ \
> --
> 2.7.4
>

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)