On 2/27/2018 5:27 PM, Jason Gunthorpe wrote:
On Tue, Feb 27, 2018 at 09:31:23AM +0200, Boris Pismenny wrote:
Hi Jason,
On 2/26/2018 8:27 PM, Jason Gunthorpe wrote:
On Sun, Feb 25, 2018 at 01:39:50PM +0200, Leon Romanovsky wrote:
static int create_raw_packet_qp_rq(struct mlx5_ib_dev *dev,
- struct mlx5_ib_rq *rq, void *qpin)
+ struct mlx5_ib_rq *rq, void *qpin,
+ int qpinlen)
{
struct mlx5_ib_qp *mqp = rq->base.container_mibqp;
__be64 *pas;
@@ -1190,6 +1191,9 @@ static int create_raw_packet_qp_rq(struct mlx5_ib_dev *dev,
int err;
u32 rq_pas_size = get_rq_pas_size(qpc);
+ if (qpinlen < 0 || (u32)qpinlen < rq_pas_size + MLX5_BYTE_OFF(create_qp_in, pas))
+ return -EINVAL;
Please use proper types instead of checking for impossible negatives.
qpinlen comes from here:
int inlen = MLX5_ST_SZ_BYTES(create_qp_in);
Which should be size_t throughout.
I definitely agree.
I've started doing it and realized that changing this here implies changes
throughout mlx5 where many types should be unsigned.
?? I don't see it. Almost all the changes are in code you added here:
What I meant to say is that once I've started doing and realized that
this changes requires too many other changes, then I've stopped and
reverted to the approach you see here.
Make qpinlen size_t in the create_raw_packet_qp_rq signature
Make inlen size_t in the create_raw_packet_qp signature
Make inlen size_t in create_qp_common
Do that and you would get warnings about passing size_t into function
that take an int argument. For example:
qpinlen is a parameter for mlx5_core_create_qp, and it passes it down
mlx5_cmd_exec. Fixing mlx5_cmd_exec requires fixing at least ~150 lines.
MLX5_ST_SZ_BYTES is already size_t
This is not the only path to set qpinlen. It could change in
create_user_qp, create_kernel_qp. For example from create_user_qp:
*inlen = MLX5_ST_SZ_BYTES(create_qp_in) +
MLX5_FLD_SZ_BYTES(create_qp_in, pas[0]) * qp->buf.npages;
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html