On Tue, Mar 3, 2020 at 10:24 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote:
>
> On 02/03/2020 05:40, Hsin-Yi Wang wrote:
> > media: mtk-vpu: avoid unaligned access to DTCM buffer.
> >
> > Previously, vpu->recv_buf and send_buf are forced cast from
> > void __iomem *tcm. vpu->recv_buf->share_buf is passed to
> > vpu_ipi_desc.handler(). It's not able to do unaligned access. Otherwise
> > kernel would crash due to unable to handle kernel paging request.
> >
> > struct vpu_run {
> > u32 signaled;
> > char fw_ver[VPU_FW_VER_LEN];
> > unsigned int dec_capability;
> > unsigned int enc_capability;
> > wait_queue_head_t wq;
> > };
> >
> > fw_ver starts at 4 byte boundary. If system enables
> > CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS, strscpy() will do
> > read_word_at_a_time(), which tries to read 8-byte: *(unsigned long *)addr
> >
> > vpu_init_ipi_handler() calls strscpy(), which would lead to crash.
> >
> > vpu_init_ipi_handler() and several other handlers (eg.
> > vpu_dec_ipi_handler) only do read access to this data, so they can be
> > const, and we can use memcpy_fromio() to copy the buf to another non iomem
> > buffer then pass to handler.
> >
> > Fixes: 85709cbf1524 ("media: replace strncpy() by strscpy()")
> > Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> > ---
> > Change in v4:
> > - Remove forced casting recv_buf from tcm. Copy iomem data before passing
> > to handler.
> > Change in v2, v3:
> > - fix sparse warnings.
> > ---
> > drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c | 9 ++--
> > .../media/platform/mtk-vcodec/vdec_vpu_if.c | 6 +--
> > .../media/platform/mtk-vcodec/venc_vpu_if.c | 12 ++---
> > drivers/media/platform/mtk-vpu/mtk_vpu.c | 45 ++++++++++---------
> > drivers/media/platform/mtk-vpu/mtk_vpu.h | 2 +-
> > 5 files changed, 38 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> > index 6720d11f50cf..dc95b8a44759 100644
> > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> > @@ -15,7 +15,7 @@ static inline struct mtk_mdp_ctx *vpu_to_ctx(struct mtk_mdp_vpu *vpu)
> > return container_of(vpu, struct mtk_mdp_ctx, vpu);
> > }
> >
> > -static void mtk_mdp_vpu_handle_init_ack(struct mdp_ipi_comm_ack *msg)
> > +static void mtk_mdp_vpu_handle_init_ack(const struct mdp_ipi_comm_ack *msg)
> > {
> > struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)
> > (unsigned long)msg->ap_inst;
> > @@ -26,10 +26,11 @@ static void mtk_mdp_vpu_handle_init_ack(struct mdp_ipi_comm_ack *msg)
> > vpu->inst_addr = msg->vpu_inst_addr;
> > }
> >
> > -static void mtk_mdp_vpu_ipi_handler(void *data, unsigned int len, void *priv)
> > +static void mtk_mdp_vpu_ipi_handler(const void *data, unsigned int len,
> > + void *priv)
> > {
> > - unsigned int msg_id = *(unsigned int *)data;
> > - struct mdp_ipi_comm_ack *msg = (struct mdp_ipi_comm_ack *)data;
> > + unsigned int msg_id = *(const unsigned int *)data;
> > + const struct mdp_ipi_comm_ack *msg = data;
>
> Why not just do:
>
> const struct mdp_ipi_comm_ack *msg = data;
> unsigned int msg_id = msg->msg_id;
>
> Much cleaner.
>
> Other than this small issue this patch looks nice. No more sparse/smatch warnings
> and no more weird casts :-)
>
> Regards,
>
> Hans
>
Updated in V5, thanks :)
https://lore.kernel.org/lkml/20200304025851.173570-1-hsinyi@xxxxxxxxxxxx/
