linux-kernel - Re: [PATCH v4 12/21] camss: vfe: Format conversion support using PIX interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdV70ajjwKTXLyyJoxNdTf_aQHjwFK6Uu+_PDHmV+Fgjyw@mail.gmail.com>
Date:   Sun, 10 Sep 2017 11:58:31 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Todor Tomov <todor.tomov@...aro.org>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Sylwester Nawrocki <s.nawrocki@...sung.com>,
        Sakari Ailus <sakari.ailus@....fi>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH v4 12/21] camss: vfe: Format conversion support using PIX interface

Hi Todor,

On Tue, Aug 8, 2017 at 3:30 PM, Todor Tomov <todor.tomov@...aro.org> wrote:
> Use VFE PIX input interface and do format conversion in VFE.
>
> Supported input format is UYVY (single plane YUV 4:2:2) and
> its different sample order variations.
>
> Supported output formats are:
> - NV12/NV21 (two plane YUV 4:2:0)
> - NV16/NV61 (two plane YUV 4:2:2)
>
> Signed-off-by: Todor Tomov <todor.tomov@...aro.org>

This is now commit 9b5833f7b82f1431 upstream.

> @@ -355,6 +471,38 @@ static void vfe_bus_disconnect_wm_from_rdi(struct vfe_device *vfe, u8 wm,
>         vfe_reg_clr(vfe, VFE_0_BUS_XBAR_CFG_x(wm), reg);
>  }
>
> +static void vfe_set_xbar_cfg(struct vfe_device *vfe, struct vfe_output *output,
> +                            u8 enable)
> +{
> +       struct vfe_line *line = container_of(output, struct vfe_line, output);
> +       u32 p = line->video_out.active_fmt.fmt.pix_mp.pixelformat;
> +       u32 reg;

With gcc 4.1.2:

    drivers/media/platform/qcom/camss-8x16/camss-vfe.c: In function
‘vfe_set_xbar_cfg’:
    drivers/media/platform/qcom/camss-8x16/camss-vfe.c:614: warning:
‘reg’ may be used uninitialized in this function

This is a false positive, as output->wm_num is always either 1 or 2, hence the
index i can never have a value different from 0 or 1, and reg is thus always
initialized.

> +       unsigned int i;
> +
> +       for (i = 0; i < output->wm_num; i++) {
> +               if (i == 0) {
> +                       reg = VFE_0_BUS_XBAR_CFG_x_M_SINGLE_STREAM_SEL_LUMA <<
> +                               VFE_0_BUS_XBAR_CFG_x_M_SINGLE_STREAM_SEL_SHIFT;
> +               } else if (i == 1) {
> +                       reg = VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_EN;
> +                       if (p == V4L2_PIX_FMT_NV12 || p == V4L2_PIX_FMT_NV16)
> +                               reg |= VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_SWAP_INTER_INTRA;
> +               }

> @@ -458,6 +728,10 @@ static void vfe_init_outputs(struct vfe_device *vfe)
>                 output->buf[0] = NULL;
>                 output->buf[1] = NULL;
>                 INIT_LIST_HEAD(&output->pending_bufs);
> +
> +               output->wm_num = 1;
> +               if (vfe->line[i].id == VFE_LINE_PIX)
> +                       output->wm_num = 2;
>         }
>  }
>

> --- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.h
> +++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.h
> @@ -30,8 +30,9 @@
>  #define MSM_VFE_PAD_SRC 1
>  #define MSM_VFE_PADS_NUM 2
>
> -#define MSM_VFE_LINE_NUM 3
> +#define MSM_VFE_LINE_NUM 4
>  #define MSM_VFE_IMAGE_MASTERS_NUM 7
> +#define MSM_VFE_COMPOSITE_IRQ_NUM 4
>
>  #define MSM_VFE_VFE0_UB_SIZE 1023
>  #define MSM_VFE_VFE0_UB_SIZE_RDI (MSM_VFE_VFE0_UB_SIZE / 3)
> @@ -51,11 +52,13 @@ enum vfe_line_id {
>         VFE_LINE_NONE = -1,
>         VFE_LINE_RDI0 = 0,
>         VFE_LINE_RDI1 = 1,
> -       VFE_LINE_RDI2 = 2
> +       VFE_LINE_RDI2 = 2,
> +       VFE_LINE_PIX = 3
>  };
>
>  struct vfe_output {
> -       u8 wm_idx;
> +       u8 wm_num;
> +       u8 wm_idx[3];

However, wm_idx[] reserves space for 3 entries, while currently only 2 are
needed. Why?

If this is meant to accommodate for a future extension, the false positive
will become a real issue.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds