On Mon, Feb 26, 2018 at 09:11:47PM +0200, Leon Romanovsky wrote: > On Mon, Feb 26, 2018 at 11:33:10AM -0700, Jason Gunthorpe wrote: > > On Sun, Feb 25, 2018 at 01:39:55PM +0200, Leon Romanovsky wrote: > > > > > + /* check multiplication overflow */ > > > + if (ucmd.cqe_size && SIZE_MAX / (size_t)ucmd.cqe_size <= entries - 1) > > > + return -EINVAL; > > > > This division is done on size_t, pretty sure the cast isn't needed. > > cqe_size is __u16 So? SIZE_MAX is size_t and division promotes the smallest argument to the largest. > > > umem = ib_umem_get(context, ucmd.buf_addr, entries * ucmd.cqe_size, > > > > But the protected multiplication is done on (int) * (u16) > > > > So this isn't going to work properly. > > It works properly because mlx5_ib_resize_cq ensure that entries > 1 and > ib_umem_get() converts entries * ucmd.cqe_size to be size_t. No, the division is done as (int) * (u16) then promoted to the size_t for the function argument, so the multiplication can still overflow. If I recall my standards right. In any event, I shouldn't have to look up minutia in a standard to see your code is right/wrong, ensure that one of the arguments is the right type and things happen correctly.. 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
