fix some issues reported by Dan, 1. fix some signedness bug 2. don't use u32 as function return value 3. prevent a divide by zero bug 4. Just return zero on success, don't return a known parameter 5. check the validity of some variables 6. reset buffer state when return buffers Signed-off-by: Ming Qian <ming.qian@xxxxxxx> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> --- drivers/media/platform/amphion/vpu_core.c | 2 +- drivers/media/platform/amphion/vpu_helpers.c | 13 +++-- drivers/media/platform/amphion/vpu_helpers.h | 6 +- drivers/media/platform/amphion/vpu_imx8q.c | 2 +- drivers/media/platform/amphion/vpu_imx8q.h | 2 +- drivers/media/platform/amphion/vpu_malone.c | 59 ++++++++++++-------- drivers/media/platform/amphion/vpu_msgs.c | 6 +- drivers/media/platform/amphion/vpu_rpc.c | 4 +- drivers/media/platform/amphion/vpu_rpc.h | 4 +- drivers/media/platform/amphion/vpu_v4l2.c | 8 ++- drivers/media/platform/amphion/vpu_windsor.c | 6 ++ 11 files changed, 69 insertions(+), 43 deletions(-) diff --git a/drivers/media/platform/amphion/vpu_core.c b/drivers/media/platform/amphion/vpu_core.c index 968b578700e3..a5dcb4abf954 100644 --- a/drivers/media/platform/amphion/vpu_core.c +++ b/drivers/media/platform/amphion/vpu_core.c @@ -472,7 +472,7 @@ struct vpu_inst *vpu_core_find_instance(struct vpu_core *core, u32 index) struct vpu_inst *tmp; mutex_lock(&core->lock); - if (!test_bit(index, &core->instance_mask)) + if (index >= core->supported_instance_count || !test_bit(index, &core->instance_mask)) goto exit; list_for_each_entry(tmp, &core->instances, list) { if (tmp->id == index) { diff --git a/drivers/media/platform/amphion/vpu_helpers.c b/drivers/media/platform/amphion/vpu_helpers.c index 768abf89e606..e9aeb3453dfc 100644 --- a/drivers/media/platform/amphion/vpu_helpers.c +++ b/drivers/media/platform/amphion/vpu_helpers.c @@ -197,7 +197,7 @@ u32 vpu_helper_get_plane_size(u32 fmt, u32 w, u32 h, int plane_no, } } -u32 vpu_helper_copy_from_stream_buffer(struct vpu_buffer *stream_buffer, +int vpu_helper_copy_from_stream_buffer(struct vpu_buffer *stream_buffer, u32 *rptr, u32 size, void *dst) { u32 offset; @@ -227,10 +227,11 @@ u32 vpu_helper_copy_from_stream_buffer(struct vpu_buffer *stream_buffer, } *rptr = vpu_helper_step_walk(stream_buffer, offset, size); - return size; + + return 0; } -u32 vpu_helper_copy_to_stream_buffer(struct vpu_buffer *stream_buffer, +int vpu_helper_copy_to_stream_buffer(struct vpu_buffer *stream_buffer, u32 *wptr, u32 size, void *src) { u32 offset; @@ -260,10 +261,10 @@ u32 vpu_helper_copy_to_stream_buffer(struct vpu_buffer *stream_buffer, *wptr = vpu_helper_step_walk(stream_buffer, offset, size); - return size; + return 0; } -u32 vpu_helper_memset_stream_buffer(struct vpu_buffer *stream_buffer, +int vpu_helper_memset_stream_buffer(struct vpu_buffer *stream_buffer, u32 *wptr, u8 val, u32 size) { u32 offset; @@ -297,7 +298,7 @@ u32 vpu_helper_memset_stream_buffer(struct vpu_buffer *stream_buffer, *wptr = offset; - return size; + return 0; } u32 vpu_helper_get_free_space(struct vpu_inst *inst) diff --git a/drivers/media/platform/amphion/vpu_helpers.h b/drivers/media/platform/amphion/vpu_helpers.h index 130d1357c032..bc28350958be 100644 --- a/drivers/media/platform/amphion/vpu_helpers.h +++ b/drivers/media/platform/amphion/vpu_helpers.h @@ -19,11 +19,11 @@ u32 vpu_helper_valid_frame_width(struct vpu_inst *inst, u32 width); u32 vpu_helper_valid_frame_height(struct vpu_inst *inst, u32 height); u32 vpu_helper_get_plane_size(u32 fmt, u32 width, u32 height, int plane_no, u32 stride, u32 interlaced, u32 *pbl); -u32 vpu_helper_copy_from_stream_buffer(struct vpu_buffer *stream_buffer, +int vpu_helper_copy_from_stream_buffer(struct vpu_buffer *stream_buffer, u32 *rptr, u32 size, void *dst); -u32 vpu_helper_copy_to_stream_buffer(struct vpu_buffer *stream_buffer, +int vpu_helper_copy_to_stream_buffer(struct vpu_buffer *stream_buffer, u32 *wptr, u32 size, void *src); -u32 vpu_helper_memset_stream_buffer(struct vpu_buffer *stream_buffer, +int vpu_helper_memset_stream_buffer(struct vpu_buffer *stream_buffer, u32 *wptr, u8 val, u32 size); u32 vpu_helper_get_free_space(struct vpu_inst *inst); u32 vpu_helper_get_used_space(struct vpu_inst *inst); diff --git a/drivers/media/platform/amphion/vpu_imx8q.c b/drivers/media/platform/amphion/vpu_imx8q.c index 606cc53125f8..f14c2b8312a8 100644 --- a/drivers/media/platform/amphion/vpu_imx8q.c +++ b/drivers/media/platform/amphion/vpu_imx8q.c @@ -165,7 +165,7 @@ int vpu_imx8q_on_firmware_loaded(struct vpu_core *core) return 0; } -u32 vpu_imx8q_check_memory_region(dma_addr_t base, dma_addr_t addr, u32 size) +int vpu_imx8q_check_memory_region(dma_addr_t base, dma_addr_t addr, u32 size) { const struct vpu_rpc_region_t imx8q_regions[] = { {0x00000000, 0x08000000, VPU_CORE_MEMORY_CACHED}, diff --git a/drivers/media/platform/amphion/vpu_imx8q.h b/drivers/media/platform/amphion/vpu_imx8q.h index d63a2747e29c..9deffd7dde42 100644 --- a/drivers/media/platform/amphion/vpu_imx8q.h +++ b/drivers/media/platform/amphion/vpu_imx8q.h @@ -108,7 +108,7 @@ int vpu_imx8q_set_system_cfg_common(struct vpu_rpc_system_config *config, u32 re int vpu_imx8q_boot_core(struct vpu_core *core); int vpu_imx8q_get_power_state(struct vpu_core *core); int vpu_imx8q_on_firmware_loaded(struct vpu_core *core); -u32 vpu_imx8q_check_memory_region(dma_addr_t base, dma_addr_t addr, u32 size); +int vpu_imx8q_check_memory_region(dma_addr_t base, dma_addr_t addr, u32 size); bool vpu_imx8q_check_codec(enum vpu_core_type type); bool vpu_imx8q_check_fmt(enum vpu_core_type type, u32 pixelfmt); diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c index d9cecbb42b2a..1212f7919957 100644 --- a/drivers/media/platform/amphion/vpu_malone.c +++ b/drivers/media/platform/amphion/vpu_malone.c @@ -1006,8 +1006,8 @@ static int vpu_malone_add_padding_scode(struct vpu_buffer *stream_buffer, u32 pixelformat, u32 scode_type) { u32 wptr; - u32 size; - u32 total_size = 0; + int size; + int total_size = 0; const struct malone_padding_scode *ps; const u32 padding_size = 4096; int ret; @@ -1024,7 +1024,7 @@ static int vpu_malone_add_padding_scode(struct vpu_buffer *stream_buffer, size = sizeof(ps->data); ret = vpu_helper_copy_to_stream_buffer(stream_buffer, &wptr, size, (void *)ps->data); - if (ret < size) + if (ret < 0) return -EINVAL; total_size += size; @@ -1234,12 +1234,15 @@ static int vpu_malone_insert_scode_seq(struct malone_scode_t *scode, u32 codec_i &scode->wptr, sizeof(hdr), hdr); - return ret; + if (ret < 0) + return ret; + return sizeof(hdr); } static int vpu_malone_insert_scode_pic(struct malone_scode_t *scode, u32 codec_id, u32 ext_size) { u8 hdr[MALONE_PAYLOAD_HEADER_SIZE]; + int ret; set_payload_hdr(hdr, SCODE_PICTURE, @@ -1247,10 +1250,13 @@ static int vpu_malone_insert_scode_pic(struct malone_scode_t *scode, u32 codec_i ext_size + vb2_get_plane_payload(scode->vb, 0), scode->inst->out_format.width, scode->inst->out_format.height); - return vpu_helper_copy_to_stream_buffer(&scode->inst->stream_buffer, - &scode->wptr, - sizeof(hdr), - hdr); + ret = vpu_helper_copy_to_stream_buffer(&scode->inst->stream_buffer, + &scode->wptr, + sizeof(hdr), + hdr); + if (ret < 0) + return ret; + return sizeof(hdr); } static int vpu_malone_insert_scode_vc1_g_pic(struct malone_scode_t *scode) @@ -1258,6 +1264,7 @@ static int vpu_malone_insert_scode_vc1_g_pic(struct malone_scode_t *scode) struct vb2_v4l2_buffer *vbuf; u8 nal_hdr[MALONE_VC1_NAL_HEADER_LEN]; u32 *data = NULL; + int ret; vbuf = to_vb2_v4l2_buffer(scode->vb); data = vb2_plane_vaddr(scode->vb, 0); @@ -1268,10 +1275,13 @@ static int vpu_malone_insert_scode_vc1_g_pic(struct malone_scode_t *scode) return 0; create_vc1_nal_pichdr(nal_hdr); - return vpu_helper_copy_to_stream_buffer(&scode->inst->stream_buffer, - &scode->wptr, - sizeof(nal_hdr), - nal_hdr); + ret = vpu_helper_copy_to_stream_buffer(&scode->inst->stream_buffer, + &scode->wptr, + sizeof(nal_hdr), + nal_hdr); + if (ret < 0) + return ret; + return sizeof(nal_hdr); } static int vpu_malone_insert_scode_vc1_l_seq(struct malone_scode_t *scode) @@ -1282,8 +1292,7 @@ static int vpu_malone_insert_scode_vc1_l_seq(struct malone_scode_t *scode) scode->need_data = 0; - ret = vpu_malone_insert_scode_seq(scode, MALONE_CODEC_ID_VC1_SIMPLE, - sizeof(rcv_seqhdr)); + ret = vpu_malone_insert_scode_seq(scode, MALONE_CODEC_ID_VC1_SIMPLE, sizeof(rcv_seqhdr)); if (ret < 0) return ret; size = ret; @@ -1299,7 +1308,7 @@ static int vpu_malone_insert_scode_vc1_l_seq(struct malone_scode_t *scode) if (ret < 0) return ret; - size += ret; + size += sizeof(rcv_seqhdr); return size; } @@ -1322,7 +1331,7 @@ static int vpu_malone_insert_scode_vc1_l_pic(struct malone_scode_t *scode) rcv_pichdr); if (ret < 0) return ret; - size += ret; + size += sizeof(rcv_pichdr); return size; } @@ -1346,7 +1355,7 @@ static int vpu_malone_insert_scode_vp8_seq(struct malone_scode_t *scode) ivf_hdr); if (ret < 0) return ret; - size += ret; + size += sizeof(ivf_hdr); return size; } @@ -1369,7 +1378,7 @@ static int vpu_malone_insert_scode_vp8_pic(struct malone_scode_t *scode) ivf_hdr); if (ret < 0) return ret; - size += ret; + size += sizeof(ivf_hdr); return size; } @@ -1470,9 +1479,9 @@ static int vpu_malone_input_frame_data(struct vpu_malone_str_buffer __iomem *str &wptr, vb2_get_plane_payload(vb, 0), vb2_plane_vaddr(vb, 0)); - if (ret < vb2_get_plane_payload(vb, 0)) + if (ret < 0) return -ENOMEM; - size += ret; + size += vb2_get_plane_payload(vb, 0); vpu_malone_update_wptr(str_buf, wptr); @@ -1500,7 +1509,7 @@ static int vpu_malone_input_stream_data(struct vpu_malone_str_buffer __iomem *st &wptr, vb2_get_plane_payload(vb, 0), vb2_plane_vaddr(vb, 0)); - if (ret < vb2_get_plane_payload(vb, 0)) + if (ret < 0) return -ENOMEM; vpu_malone_update_wptr(str_buf, wptr); @@ -1566,9 +1575,13 @@ static bool vpu_malone_check_ready(struct vpu_shared_addr *shared, u32 instance) u32 size = desc->end - desc->start; u32 rptr = desc->rptr; u32 wptr = desc->wptr; - u32 used = (wptr + size - rptr) % size; + u32 used; + + if (!size) + return true; - if (!size || used < (size >> 1)) + used = (wptr + size - rptr) % size; + if (used < (size >> 1)) return true; return false; diff --git a/drivers/media/platform/amphion/vpu_msgs.c b/drivers/media/platform/amphion/vpu_msgs.c index 68df43913904..58502c51ddb3 100644 --- a/drivers/media/platform/amphion/vpu_msgs.c +++ b/drivers/media/platform/amphion/vpu_msgs.c @@ -214,7 +214,7 @@ static int vpu_session_handle_msg(struct vpu_inst *inst, struct vpu_rpc_event *m static bool vpu_inst_receive_msg(struct vpu_inst *inst, struct vpu_rpc_event *pkt) { - u32 bytes = sizeof(struct vpu_rpc_event_header); + unsigned long bytes = sizeof(struct vpu_rpc_event_header); u32 ret; memset(pkt, 0, sizeof(*pkt)); @@ -246,7 +246,7 @@ void vpu_inst_run_work(struct work_struct *work) static void vpu_inst_handle_msg(struct vpu_inst *inst, struct vpu_rpc_event *pkt) { - u32 bytes; + unsigned long bytes; u32 id = pkt->hdr.id; int ret; @@ -337,7 +337,7 @@ void vpu_msg_delayed_work(struct work_struct *work) { struct vpu_core *core; struct delayed_work *dwork; - u32 bytes = sizeof(bytes); + unsigned long bytes = sizeof(u32); u32 i; if (!work) diff --git a/drivers/media/platform/amphion/vpu_rpc.c b/drivers/media/platform/amphion/vpu_rpc.c index 6e01abaa5d16..18a164766409 100644 --- a/drivers/media/platform/amphion/vpu_rpc.c +++ b/drivers/media/platform/amphion/vpu_rpc.c @@ -20,7 +20,7 @@ #include "vpu_windsor.h" #include "vpu_malone.h" -u32 vpu_iface_check_memory_region(struct vpu_core *core, dma_addr_t addr, u32 size) +int vpu_iface_check_memory_region(struct vpu_core *core, dma_addr_t addr, u32 size) { struct vpu_iface_ops *ops = vpu_core_get_iface(core); @@ -63,6 +63,8 @@ static int vpu_rpc_send_cmd_buf(struct vpu_shared_addr *shared, struct vpu_rpc_e u32 wptr; u32 i; + if (cmd->hdr.num > 0xff || cmd->hdr.num >= ARRAY_SIZE(cmd->data)) + return -EINVAL; desc = shared->cmd_desc; space = vpu_rpc_check_buffer_space(desc, true); if (space < (((cmd->hdr.num + 1) << 2) + 16)) diff --git a/drivers/media/platform/amphion/vpu_rpc.h b/drivers/media/platform/amphion/vpu_rpc.h index c764ff52d026..5ea4f8aff846 100644 --- a/drivers/media/platform/amphion/vpu_rpc.h +++ b/drivers/media/platform/amphion/vpu_rpc.h @@ -43,7 +43,7 @@ struct vpu_iface_ops { bool (*check_codec)(enum vpu_core_type type); bool (*check_fmt)(enum vpu_core_type type, u32 pixelfmt); u32 (*get_data_size)(void); - u32 (*check_memory_region)(dma_addr_t base, dma_addr_t addr, u32 size); + int (*check_memory_region)(dma_addr_t base, dma_addr_t addr, u32 size); int (*boot_core)(struct vpu_core *core); int (*shutdown_core)(struct vpu_core *core); int (*restore_core)(struct vpu_core *core); @@ -113,7 +113,7 @@ struct vpu_rpc_region_t { struct vpu_iface_ops *vpu_core_get_iface(struct vpu_core *core); struct vpu_iface_ops *vpu_inst_get_iface(struct vpu_inst *inst); -u32 vpu_iface_check_memory_region(struct vpu_core *core, dma_addr_t addr, u32 size); +int vpu_iface_check_memory_region(struct vpu_core *core, dma_addr_t addr, u32 size); static inline bool vpu_iface_check_codec(struct vpu_core *core) { diff --git a/drivers/media/platform/amphion/vpu_v4l2.c b/drivers/media/platform/amphion/vpu_v4l2.c index 6fe077a685e8..9c0704cd5766 100644 --- a/drivers/media/platform/amphion/vpu_v4l2.c +++ b/drivers/media/platform/amphion/vpu_v4l2.c @@ -403,11 +403,15 @@ void vpu_vb2_buffers_return(struct vpu_inst *inst, unsigned int type, enum vb2_b struct vb2_v4l2_buffer *buf; if (V4L2_TYPE_IS_OUTPUT(type)) { - while ((buf = v4l2_m2m_src_buf_remove(inst->fh.m2m_ctx))) + while ((buf = v4l2_m2m_src_buf_remove(inst->fh.m2m_ctx))) { + vpu_set_buffer_state(buf, VPU_BUF_STATE_IDLE); v4l2_m2m_buf_done(buf, state); + } } else { - while ((buf = v4l2_m2m_dst_buf_remove(inst->fh.m2m_ctx))) + while ((buf = v4l2_m2m_dst_buf_remove(inst->fh.m2m_ctx))) { + vpu_set_buffer_state(buf, VPU_BUF_STATE_IDLE); v4l2_m2m_buf_done(buf, state); + } } } diff --git a/drivers/media/platform/amphion/vpu_windsor.c b/drivers/media/platform/amphion/vpu_windsor.c index a056ad624e9b..1526af2ef9da 100644 --- a/drivers/media/platform/amphion/vpu_windsor.c +++ b/drivers/media/platform/amphion/vpu_windsor.c @@ -818,12 +818,18 @@ int vpu_windsor_config_memory_resource(struct vpu_shared_addr *shared, switch (type) { case MEM_RES_ENC: + if (index >= ARRAY_SIZE(pool->enc_frames)) + return -EINVAL; res = &pool->enc_frames[index]; break; case MEM_RES_REF: + if (index >= ARRAY_SIZE(pool->ref_frames)) + return -EINVAL; res = &pool->ref_frames[index]; break; case MEM_RES_ACT: + if (index) + return -EINVAL; res = &pool->act_frame; break; default: -- 2.33.0