On Thu, Feb 24, 2022 at 11:10:08AM +0800, Ming Qian wrote:
> +static int vpu_malone_add_padding_scode(struct vpu_buffer *stream_buffer,
> + struct vpu_malone_str_buffer __iomem *str_buf,
> + u32 pixelformat, u32 scode_type)
> +{
> + u32 wptr;
> + u32 size;
> + u32 total_size = 0;
> + const struct malone_padding_scode *ps;
> + const u32 padding_size = 4096;
> + int ret;
> +
> + ps = get_padding_scode(scode_type, pixelformat);
> + if (!ps)
> + return -EINVAL;
> +
> + wptr = readl(&str_buf->wptr);
> + size = ALIGN(wptr, 4) - wptr;
The ALIGN() macro can wrap to zero if wptr is > UINT_MAX - 4. This
would make size into a very high unsigned value.
> + if (size)
> + vpu_helper_memset_stream_buffer(stream_buffer, &wptr, 0, size);
> + total_size += size;
> +
> + size = sizeof(ps->data);
> + ret = vpu_helper_copy_to_stream_buffer(stream_buffer, &wptr, size, (void *)ps->data);
> + if (ret < size)
The problem here is that size is a u32 so ret is type promoted to a u32
and (u32)-EINVAL > size so the condition is impossible.
> + return -EINVAL;
> + total_size += size;
> +
> + size = padding_size - sizeof(ps->data);
> + vpu_helper_memset_stream_buffer(stream_buffer, &wptr, 0, size);
> + total_size += size;
> +
> + vpu_malone_update_wptr(str_buf, wptr);
> + return total_size;
What was the point of making total_size a u32 if the function itself is
and int?
> +}
[ snip ]
> +static int vpu_malone_input_frame_data(struct vpu_malone_str_buffer __iomem *str_buf,
> + struct vpu_inst *inst, struct vb2_buffer *vb,
> + u32 disp_imm)
> +{
> + struct malone_scode_t scode;
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + u32 wptr = readl(&str_buf->wptr);
> + int size = 0;
> + int ret = 0;
> +
> + /*add scode: SCODE_SEQUENCE, SCODE_PICTURE, SCODE_SLICE*/
> + scode.inst = inst;
> + scode.vb = vb;
> + scode.wptr = wptr;
> + scode.need_data = 1;
> + if (vbuf->sequence == 0 || vpu_vb_is_codecconfig(vbuf))
> + ret = vpu_malone_insert_scode(&scode, SCODE_SEQUENCE);
> +
> + if (ret < 0)
> + return -ENOMEM;
> + size += ret;
> + wptr = scode.wptr;
> + if (!scode.need_data) {
> + vpu_malone_update_wptr(str_buf, wptr);
> + return size;
> + }
> +
> + ret = vpu_malone_insert_scode(&scode, SCODE_PICTURE);
> + if (ret < 0)
> + return -ENOMEM;
> + size += ret;
> + wptr = scode.wptr;
> +
> + ret = vpu_helper_copy_to_stream_buffer(&inst->stream_buffer,
> + &wptr,
> + vb2_get_plane_payload(vb, 0),
> + vb2_plane_vaddr(vb, 0));
> + if (ret < vb2_get_plane_payload(vb, 0))
Here again, negative values of "ret" are type promoted to high unsigned
values so the condition is impossible.
> + return -ENOMEM;
> + size += ret;
> +
> + vpu_malone_update_wptr(str_buf, wptr);
> +
> + if (disp_imm && !vpu_vb_is_codecconfig(vbuf)) {
> + ret = vpu_malone_add_scode(inst->core->iface,
> + inst->id,
> + &inst->stream_buffer,
> + inst->out_format.pixfmt,
> + SCODE_PADDING_BUFFLUSH);
> + if (ret < 0)
> + return ret;
> + size += ret;
> + }
> +
> + return size;
> +}
> +
> +static int vpu_malone_input_stream_data(struct vpu_malone_str_buffer __iomem *str_buf,
> + struct vpu_inst *inst, struct vb2_buffer *vb)
> +{
> + u32 wptr = readl(&str_buf->wptr);
> + int ret = 0;
> +
> + ret = vpu_helper_copy_to_stream_buffer(&inst->stream_buffer,
> + &wptr,
> + vb2_get_plane_payload(vb, 0),
> + vb2_plane_vaddr(vb, 0));
> + if (ret < vb2_get_plane_payload(vb, 0))
Same thing. This condition is impossible.
> + return -ENOMEM;
> +
> + vpu_malone_update_wptr(str_buf, wptr);
> +
> + return ret;
> +}
regards,
dan carpenter
