Re: [PATCH rdma-rc 3/9] IB/mlx5: Fix out-of-bounds read in create_raw_packet_qp_rq

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux