1. 17 Jun, 2021 27 commits
    • Wolfram Sang's avatar
      media: coda: set debugfs blobs to read only · ca816468
      Wolfram Sang authored
      Those blobs can only be read. So, don't confuse users with 'writable'
      flags.
      Signed-off-by: default avatarWolfram Sang <wsa+renesas@sang-engineering.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      ca816468
    • Arnd Bergmann's avatar
      media: subdev: disallow ioctl for saa6588/davinci · 0a7790be
      Arnd Bergmann authored
      The saa6588_ioctl() function expects to get called from other kernel
      functions with a 'saa6588_command' pointer, but I found nothing stops it
      from getting called from user space instead, which seems rather dangerous.
      
      The same thing happens in the davinci vpbe driver with its VENC_GET_FLD
      command.
      
      As a quick fix, add a separate .command() callback pointer for this
      driver and change the two callers over to that.  This change can easily
      get backported to stable kernels if necessary, but since there are only
      two drivers, we may want to eventually replace this with a set of more
      specialized callbacks in the long run.
      
      Fixes: c3fda7f8 ("V4L/DVB (10537): saa6588: convert to v4l2_subdev.")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      0a7790be
    • Arnd Bergmann's avatar
      media: atomisp: remove compat_ioctl32 code · b4c650f1
      Arnd Bergmann authored
      This is one of the last remaining users of compat_alloc_user_space()
      and copy_in_user(), which are in the process of getting removed.
      
      As of commit 57e6b6f2 ("media: atomisp_fops.c: disable
      atomisp_compat_ioctl32"), nothing in this file is actually getting used
      as the only reference has been stubbed out.
      
      Remove the entire file -- anyone willing to restore the functionality
      can equally well just look up the contents in the git history if needed.
      Acked-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
      Suggested-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      Acked-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      b4c650f1
    • Arnd Bergmann's avatar
      media: v4l2-core: return -ENODEV from ioctl when not registered · 8162f78d
      Arnd Bergmann authored
      I spotted a minor difference is handling of unregistered devices
      between native and compat ioctls: the native handler never tries
      to call into the driver if a device is not marked as registered.
      
      I did not check whether this can cause issues in the kernel, or
      just a different between return codes, but it clearly makes
      sense that both should behave the same way.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      8162f78d
    • Arnd Bergmann's avatar
      media: subdev: remove VIDIOC_DQEVENT_TIME32 handling · 765ba251
      Arnd Bergmann authored
      Converting the VIDIOC_DQEVENT_TIME32/VIDIOC_DQEVENT32/
      VIDIOC_DQEVENT32_TIME32 arguments to the canonical form is done in common
      code, but for some reason I ended up adding another conversion helper to
      subdev_do_ioctl() as well. I must have concluded that this does not go
      through the common conversion, but it has done that since the ioctl
      handler was first added.
      
      I assume this one is harmless as there should be no way to arrive here
      from user space if CONFIG_COMPAT_32BIT_TIME is set, but since it is dead
      code, it should just get removed.
      
      On a 64-bit architecture, as well as a 32-bit architecture without
      CONFIG_COMPAT_32BIT_TIME, handling this command is a mistake,
      and the kernel should return an error.
      
      Fixes: 1a6c0b36 ("media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      765ba251
    • Arnd Bergmann's avatar
      media: v4l2-core: fix whitespace damage in video_get_user() · e84c8932
      Arnd Bergmann authored
      The initialization was indented with an extra tab in most lines,
      remove them to get the normal coding style.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      e84c8932
    • Arnd Bergmann's avatar
      media: v4l2-core: explicitly clear ioctl input data · 7b53cca7
      Arnd Bergmann authored
      As seen from a recent syzbot bug report, mistakes in the compat ioctl
      implementation can lead to uninitialized kernel stack data getting used
      as input for driver ioctl handlers.
      
      The reported bug is now fixed, but it's possible that other related
      bugs are still present or get added in the future. As the drivers need
      to check user input already, the possible impact is fairly low, but it
      might still cause an information leak.
      
      To be on the safe side, always clear the entire ioctl buffer before
      calling the conversion handler functions that are meant to initialize
      them.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      7b53cca7
    • Arnd Bergmann's avatar
      media: v4l2-core: ignore native time32 ioctls on 64-bit · c344f07a
      Arnd Bergmann authored
      Syzbot found that passing ioctl command 0xc0505609 into a 64-bit
      kernel from a 32-bit process causes uninitialized kernel memory to
      get passed to drivers instead of the user space data:
      
      BUG: KMSAN: uninit-value in check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
      BUG: KMSAN: uninit-value in video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
      CPU: 0 PID: 19595 Comm: syz-executor.4 Not tainted 5.11.0-rc7-syzkaller #0
      Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
      Call Trace:
       __dump_stack lib/dump_stack.c:79 [inline]
       dump_stack+0x21c/0x280 lib/dump_stack.c:120
       kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118
       __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197
       check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
       video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
       video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391
       v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360
       v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248
       __do_compat_sys_ioctl fs/ioctl.c:842 [inline]
       __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793
       __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793
       do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline]
       __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141
       do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166
       do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209
       entry_SYSENTER_compat_after_hwframe+0x4d/0x5c
      
      The time32 commands are defined but were never meant to be called on
      64-bit machines, as those have always used time64 interfaces.  I missed
      this in my patch that introduced the time64 handling on 32-bit platforms.
      
      The problem in this case is the mismatch of one function checking for
      the numeric value of the command and another function checking for the
      type of process (native vs compat) instead, with the result being that
      for this combination, nothing gets copied into the buffer at all.
      
      Avoid this by only trying to convert the time32 commands when running
      on a 32-bit kernel where these are defined in a meaningful way.
      
      [hverkuil: fix 3 warnings: switch with no cases]
      
      Fixes: 577c89b0 ("media: v4l2-core: fix v4l2_buffer handling for time64 ABI")
      Reported-by: syzbot+142888ffec98ab194028@syzkaller.appspotmail.com
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      c344f07a
    • Irui Wang's avatar
      media: mtk-vcodec: Support MT8192 H264 4K encoding · caf231ac
      Irui Wang authored
      MT8192 H264 support 4k(3840x2176) and Level 5.1 encoding,
      add related path according to enc_capability.
      Signed-off-by: default avatarIrui Wang <irui.wang@mediatek.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      caf231ac
    • Irui Wang's avatar
      media: mtk-vcodec: Add MT8192 H264 venc driver · 37eeacba
      Irui Wang authored
      Add MT8192 venc driver's compatible and device private data.
      Reviewed-by: default avatarTzung-Bi Shih <tzungbi@google.com>
      Signed-off-by: default avatarIrui Wang <irui.wang@mediatek.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      37eeacba
    • Irui Wang's avatar
      media: dt-bindings: media: mtk-vcodec: Add binding for MT8192 VENC · aa950d86
      Irui Wang authored
      Updates binding document for mt8192 encoder driver.
      Acked-by: default avatarRob Herring <robh@kernel.org>
      Signed-off-by: default avatarIrui Wang <irui.wang@mediatek.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      aa950d86
    • Irui Wang's avatar
      media: mtk-vcodec: Support 34bits dma address for venc · c2c3bde0
      Irui Wang authored
      Use the dma_set_mask_and_coherent helper to set venc
      DMA bit mask to support 34bits iova space(16GB) that
      the mt8192 iommu HW support.
      
      Whole the iova range separate to 0~4G/4G~8G/8G~12G/12G~16G,
      regarding which iova range VENC actually locate, it
      depends on the dma-ranges property of venc dtsi node.
      Signed-off-by: default avatarIrui Wang <irui.wang@mediatek.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      c2c3bde0
    • Irui Wang's avatar
      media: dt-bindings: media: mtk-vcodec: Add dma-ranges property · 5cd57605
      Irui Wang authored
      The mt8192 iommu support 0~16GB iova. We separate it to four banks:
      0~4G; 4G~8G; 8G~12G; 12G~16G.
      
      The "dma-ranges" could be used to adjust the bank we locate.
      If we don't set this property. The default range always is 0~4G.
      
      This is optional and only needed in mt8192, the dma ranges should
      not cross 4G/8G/12G.
      
      Here we don't have actual bus/parent concept here.  And the iova
      requirement is for our HW. Thus put the property in our node.
      Acked-by: default avatarRob Herring <robh@kernel.org>
      Signed-off-by: default avatarIrui Wang <irui.wang@mediatek.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      5cd57605
    • Alexandre Courbot's avatar
      media: mtk-vcodec: venc: remove redundant code · b6c57d31
      Alexandre Courbot authored
      vidioc_try_fmt() does clamp height and width when called on the OUTPUT
      queue, so clamping them prior to calling this function is redundant. Set
      the queue's parameters after calling vidioc_try_fmt() so we can use the
      values it computed.
      Signed-off-by: default avatarAlexandre Courbot <acourbot@chromium.org>
      Signed-off-by: default avatarIrui Wang <irui.wang@mediatek.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      b6c57d31
    • Tomi Valkeinen's avatar
      media: v4l2-subdev: add subdev-wide state struct · 0d346d2a
      Tomi Valkeinen authored
      We have 'struct v4l2_subdev_pad_config' which contains configuration for
      a single pad used for the TRY functionality, and an array of those
      structs is passed to various v4l2_subdev_pad_ops.
      
      I was working on subdev internal routing between pads, and realized that
      there's no way to add TRY functionality for routes, which is not pad
      specific configuration. Adding a separate struct for try-route config
      wouldn't work either, as e.g. set-fmt needs to know the try-route
      configuration to propagate the settings.
      
      This patch adds a new struct, 'struct v4l2_subdev_state' (which at the
      moment only contains the v4l2_subdev_pad_config array) and the new
      struct is used in most of the places where v4l2_subdev_pad_config was
      used. All v4l2_subdev_pad_ops functions taking v4l2_subdev_pad_config
      are changed to instead take v4l2_subdev_state.
      
      The changes to drivers/media/v4l2-core/v4l2-subdev.c and
      include/media/v4l2-subdev.h were written by hand, and all the driver
      changes were done with the semantic patch below. The spatch needs to be
      applied to a select list of directories. I used the following shell
      commands to apply the spatch:
      
      dirs="drivers/media/i2c drivers/media/platform drivers/media/usb drivers/media/test-drivers/vimc drivers/media/pci drivers/staging/media"
      for dir in $dirs; do spatch -j8 --dir --include-headers --no-show-diff --in-place --sp-file v4l2-subdev-state.cocci $dir; done
      
      Note that Coccinelle chokes on a few drivers (gcc extensions?). With
      minor changes we can make Coccinelle run fine, and these changes can be
      reverted after spatch. The diff for these changes is:
      
      For drivers/media/i2c/s5k5baf.c:
      
      	@@ -1481,7 +1481,7 @@ static int s5k5baf_set_selection(struct v4l2_subdev *sd,
      	 				&s5k5baf_cis_rect,
      	 				v4l2_subdev_get_try_crop(sd, cfg, PAD_CIS),
      	 				v4l2_subdev_get_try_compose(sd, cfg, PAD_CIS),
      	-				v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT)
      	+				v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT),
      	 			};
      	 		s5k5baf_set_rect_and_adjust(rects, rtype, &sel->r);
      	 		return 0;
      
      For drivers/media/platform/s3c-camif/camif-capture.c:
      
      	@@ -1230,7 +1230,7 @@ static int s3c_camif_subdev_get_fmt(struct v4l2_subdev *sd,
      	 		*mf = camif->mbus_fmt;
      	 		break;
      
      	-	case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
      	+	case CAMIF_SD_PAD_SOURCE_C:
      	 		/* crop rectangle at camera interface input */
      	 		mf->width = camif->camif_crop.width;
      	 		mf->height = camif->camif_crop.height;
      	@@ -1332,7 +1332,7 @@ static int s3c_camif_subdev_set_fmt(struct v4l2_subdev *sd,
      	 		}
      	 		break;
      
      	-	case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
      	+	case CAMIF_SD_PAD_SOURCE_C:
      	 		/* Pixel format can be only changed on the sink pad. */
      	 		mf->code = camif->mbus_fmt.code;
      	 		mf->width = crop->width;
      
      The semantic patch is:
      
      // <smpl>
      
      // Change function parameter
      
      @@
      identifier func;
      identifier cfg;
      @@
      
       func(...,
      -   struct v4l2_subdev_pad_config *cfg
      +   struct v4l2_subdev_state *sd_state
          , ...)
       {
       <...
      - cfg
      + sd_state
       ...>
       }
      
      // Change function declaration parameter
      
      @@
      identifier func;
      identifier cfg;
      type T;
      @@
      T func(...,
      -   struct v4l2_subdev_pad_config *cfg
      +   struct v4l2_subdev_state *sd_state
          , ...);
      
      // Change function return value
      
      @@
      identifier func;
      @@
      - struct v4l2_subdev_pad_config
      + struct v4l2_subdev_state
       *func(...)
       {
          ...
       }
      
      // Change function declaration return value
      
      @@
      identifier func;
      @@
      - struct v4l2_subdev_pad_config
      + struct v4l2_subdev_state
       *func(...);
      
      // Some drivers pass a local pad_cfg for a single pad to a called function. Wrap it
      // inside a pad_state.
      
      @@
      identifier func;
      identifier pad_cfg;
      @@
      func(...)
      {
          ...
          struct v4l2_subdev_pad_config pad_cfg;
      +   struct v4l2_subdev_state pad_state = { .pads = &pad_cfg };
      
          <+...
      
      (
          v4l2_subdev_call
      |
          sensor_call
      |
          isi_try_fse
      |
          isc_try_fse
      |
          saa_call_all
      )
          (...,
      -   &pad_cfg
      +   &pad_state
          ,...)
      
          ...+>
      }
      
      // If the function uses fields from pad_config, access via state->pads
      
      @@
      identifier func;
      identifier state;
      @@
       func(...,
          struct v4l2_subdev_state *state
          , ...)
       {
          <...
      (
      -   state->try_fmt
      +   state->pads->try_fmt
      |
      -   state->try_crop
      +   state->pads->try_crop
      |
      -   state->try_compose
      +   state->pads->try_compose
      )
          ...>
      }
      
      // If the function accesses the filehandle, use fh->state instead
      
      @@
      struct v4l2_subdev_fh *fh;
      @@
      -    fh->pad
      +    fh->state
      
      @@
      struct v4l2_subdev_fh fh;
      @@
      -    fh.pad
      +    fh.state
      
      // Start of vsp1 specific
      
      @@
      @@
      struct vsp1_entity {
          ...
      -    struct v4l2_subdev_pad_config *config;
      +    struct v4l2_subdev_state *config;
          ...
      };
      
      @@
      symbol entity;
      @@
      vsp1_entity_init(...)
      {
          ...
          entity->config =
      -    v4l2_subdev_alloc_pad_config
      +    v4l2_subdev_alloc_state
          (&entity->subdev);
          ...
      }
      
      @@
      symbol entity;
      @@
      vsp1_entity_destroy(...)
      {
          ...
      -   v4l2_subdev_free_pad_config
      +   v4l2_subdev_free_state
          (entity->config);
          ...
      }
      
      @exists@
      identifier func =~ "(^vsp1.*)|(hsit_set_format)|(sru_enum_frame_size)|(sru_set_format)|(uif_get_selection)|(uif_set_selection)|(uds_enum_frame_size)|(uds_set_format)|(brx_set_format)|(brx_get_selection)|(histo_get_selection)|(histo_set_selection)|(brx_set_selection)";
      symbol config;
      @@
      func(...) {
          ...
      -    struct v4l2_subdev_pad_config *config;
      +    struct v4l2_subdev_state *config;
          ...
      }
      
      // End of vsp1 specific
      
      // Start of rcar specific
      
      @@
      identifier sd;
      identifier pad_cfg;
      @@
       rvin_try_format(...)
       {
          ...
      -   struct v4l2_subdev_pad_config *pad_cfg;
      +   struct v4l2_subdev_state *sd_state;
          ...
      -   pad_cfg = v4l2_subdev_alloc_pad_config(sd);
      +   sd_state = v4l2_subdev_alloc_state(sd);
          <...
      -   pad_cfg
      +   sd_state
          ...>
      -   v4l2_subdev_free_pad_config(pad_cfg);
      +   v4l2_subdev_free_state(sd_state);
          ...
       }
      
      // End of rcar specific
      
      // Start of rockchip specific
      
      @@
      identifier func =~ "(rkisp1_rsz_get_pad_fmt)|(rkisp1_rsz_get_pad_crop)|(rkisp1_rsz_register)";
      symbol rsz;
      symbol pad_cfg;
      @@
      
       func(...)
       {
      +   struct v4l2_subdev_state state = { .pads = rsz->pad_cfg };
          ...
      -   rsz->pad_cfg
      +   &state
          ...
       }
      
      @@
      identifier func =~ "(rkisp1_isp_get_pad_fmt)|(rkisp1_isp_get_pad_crop)";
      symbol isp;
      symbol pad_cfg;
      @@
      
       func(...)
       {
      +   struct v4l2_subdev_state state = { .pads = isp->pad_cfg };
          ...
      -   isp->pad_cfg
      +   &state
          ...
       }
      
      @@
      symbol rkisp1;
      symbol isp;
      symbol pad_cfg;
      @@
      
       rkisp1_isp_register(...)
       {
      +   struct v4l2_subdev_state state = { .pads = rkisp1->isp.pad_cfg };
          ...
      -   rkisp1->isp.pad_cfg
      +   &state
          ...
       }
      
      // End of rockchip specific
      
      // Start of tegra-video specific
      
      @@
      identifier sd;
      identifier pad_cfg;
      @@
       __tegra_channel_try_format(...)
       {
          ...
      -   struct v4l2_subdev_pad_config *pad_cfg;
      +   struct v4l2_subdev_state *sd_state;
          ...
      -   pad_cfg = v4l2_subdev_alloc_pad_config(sd);
      +   sd_state = v4l2_subdev_alloc_state(sd);
          <...
      -   pad_cfg
      +   sd_state
          ...>
      -   v4l2_subdev_free_pad_config(pad_cfg);
      +   v4l2_subdev_free_state(sd_state);
          ...
       }
      
      @@
      identifier sd_state;
      @@
       __tegra_channel_try_format(...)
       {
          ...
          struct v4l2_subdev_state *sd_state;
          <...
      -   sd_state->try_crop
      +   sd_state->pads->try_crop
          ...>
       }
      
      // End of tegra-video specific
      
      // </smpl>
      Signed-off-by: default avatarTomi Valkeinen <tomi.valkeinen@ideasonboard.com>
      Acked-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Acked-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
      Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      0d346d2a
    • Gustavo A. R. Silva's avatar
      media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members · 6f2f49ae
      Gustavo A. R. Silva authored
      There is a regular need in the kernel to provide a way to declare having
      a dynamically sized set of trailing elements in a structure. Kernel code
      should always use “flexible array members”[1] for these cases. The older
      style of one-element or zero-length arrays should no longer be used[2].
      
      Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
      hfi_msg_session_property_info_pkt instead of one-element arrays, and
      refactor the code accordingly.
      
      Also, this helps with the ongoing efforts to enable -Warray-bounds by
      fixing the following warnings:
      
        CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
      drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
      drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
        246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
            |                          ~~~~~~~~~^~~
      drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
      drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
        342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
            |                                                     ~~~~~~~~~^~~
      
      [1] https://en.wikipedia.org/wiki/Flexible_array_member
      [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
      
      Link: https://github.com/KSPP/linux/issues/79
      Link: https://github.com/KSPP/linux/issues/109Co-developed-by: default avatarKees Cook <keescook@chromium.org>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Signed-off-by: default avatarGustavo A. R. Silva <gustavoars@kernel.org>
      Signed-off-by: default avatarStanimir Varbanov <stanimir.varbanov@linaro.org>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      6f2f49ae
    • Gustavo A. R. Silva's avatar
      media: venus: hfi_cmds: Fix packet size calculation · c73c23f3
      Gustavo A. R. Silva authored
      Now that a one-element array was replaced with a flexible-array member
      in struct hfi_sys_set_property_pkt, use the struct_size() helper to
      correctly calculate the packet size.
      
      Fixes: 701e10b3fd9f ("media: venus: hfi_cmds.h: Replace one-element array with flexible-array member")
      Signed-off-by: default avatarGustavo A. R. Silva <gustavoars@kernel.org>
      Signed-off-by: default avatarStanimir Varbanov <stanimir.varbanov@linaro.org>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      c73c23f3
    • Mauro Carvalho Chehab's avatar
      media: xilinx: simplify get fourcc logic · 12891698
      Mauro Carvalho Chehab authored
      Right now, there are two calls for xvip_get_format_by_fourcc().
      If the first one fails, it is called again in order to pick
      the first available format: V4L2_PIX_FMT_YUYV.
      
      This ends by producing a smatch warnings:
      	drivers/media/platform/xilinx/xilinx-dma.c:555 __xvip_dma_try_format() error: 'info' dereferencing possible ERR_PTR()
      	drivers/media/platform/xilinx/xilinx-dma.c: drivers/media/platform/xilinx/xilinx-dma.c:664 xvip_dma_init() error: 'dma->fmtinfo' dereferencing possible ERR_PTR()
      
      as it is hard for an static analyzer to ensure that calling
      xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT) won't return an
      error.
      
      So, better to optimize the logic, ensuring that the function
      will never return an error.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      12891698
    • Mauro Carvalho Chehab's avatar
      media: dvb-core: frontend: make GET/SET safer · 60f0618d
      Mauro Carvalho Chehab authored
      The implementation for FE_SET_PROPERTY/FE_GET_PROPERTY has
      a debug code that might be explored via spectre.
      Improve the logic in order to mitigate such risk.
      
      It should be noticed that, before this patch, the logic
      which implements FE_GET_PROPERTY doesn't check the length passed
      by the user, which might lead to expose some information. This
      is probably not exploitable, though, as the frontend drivers
      won't rely on the buffer length value set by userspace, but
      it helps to return a valid value back to userspace.
      
      The code was changed to only try to access an array based on
      userspace values only when DVB debug is turned on, helping to
      reduce the attack surface, as a speculation attack would work
      only if DVB dev_dbg() macros are enabled, which is usually
      enabled only on test Kernels or by the root user.
      
      As a side effect, a const array size can now be reduced by
      ~570 bytes, as it now needs to contain just the name of each
      DTV command.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      60f0618d
    • Mauro Carvalho Chehab's avatar
      media: ttusb-dec: cleanup an error handling logic · dba328ba
      Mauro Carvalho Chehab authored
      Simplify the logic at ttusb_dec_send_command().
      
      Besides avoiding some code duplication, as a side effect,
      this could remove this false positive return with spatch:
      
      	drivers/media/usb/ttusb-dec/ttusb_dec.c:380 ttusb_dec_send_command() warn: inconsistent returns '&dec->usb_mutex'.
      	  Locked on  : 330
      	  Unlocked on: 354,365,380
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      dba328ba
    • Mauro Carvalho Chehab's avatar
      media: siano: fix device register error path · 5368b1ee
      Mauro Carvalho Chehab authored
      As reported by smatch:
      	drivers/media/common/siano/smsdvb-main.c:1231 smsdvb_hotplug() warn: '&client->entry' not removed from list
      
      If an error occur at the end of the registration logic, it won't
      drop the device from the list.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      5368b1ee
    • Mauro Carvalho Chehab's avatar
      media: saa7134: fix saa7134_initdev error handling logic · 235406dc
      Mauro Carvalho Chehab authored
      Smatch reported an issue there:
      	drivers/media/pci/saa7134/saa7134-core.c:1302 saa7134_initdev() warn: '&dev->devlist' not removed from list
      
      But besides freeing the list, the media controller graph also
      needs to be cleaned up on errors. Address those issues.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      235406dc
    • Mauro Carvalho Chehab's avatar
      media: saa7134: use more meaninful goto labels · 7f9197f1
      Mauro Carvalho Chehab authored
      Instead of just numbering fail0 to fail4, use more meaninful
      goto labels.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      7f9197f1
    • Mauro Carvalho Chehab's avatar
      media: sun6i-csi: add a missing return code · ba913911
      Mauro Carvalho Chehab authored
      As pointed by smatch, there's a missing return code:
      
      	drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c:485 sun6i_video_open() warn: missing error code 'ret'
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      ba913911
    • Mauro Carvalho Chehab's avatar
      media: dvbdev: fix error logic at dvb_register_device() · 1fec2ecc
      Mauro Carvalho Chehab authored
      As reported by smatch:
      
      	drivers/media/dvb-core/dvbdev.c: drivers/media/dvb-core/dvbdev.c:510 dvb_register_device() warn: '&dvbdev->list_head' not removed from list
      	drivers/media/dvb-core/dvbdev.c: drivers/media/dvb-core/dvbdev.c:530 dvb_register_device() warn: '&dvbdev->list_head' not removed from list
      	drivers/media/dvb-core/dvbdev.c: drivers/media/dvb-core/dvbdev.c:545 dvb_register_device() warn: '&dvbdev->list_head' not removed from list
      
      The error logic inside dvb_register_device() doesn't remove
      devices from the dvb_adapter_list in case of errors.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      1fec2ecc
    • Mauro Carvalho Chehab's avatar
      media: dvb_net: avoid speculation from net slot · abc0226d
      Mauro Carvalho Chehab authored
      The risk of especulation is actually almost-non-existing here,
      as there are very few users of TCP/IP using the DVB stack,
      as, this is mainly used with DVB-S/S2 cards, and only by people
      that receives TCP/IP from satellite connections, which limits
      a lot the number of users of such feature(*).
      
      (*) In thesis, DVB-C cards could also benefit from it, but I'm
      yet to see a hardware that supports it.
      
      Yet, fixing it is trivial.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      abc0226d
    • Mauro Carvalho Chehab's avatar
      media: dvb_ca_en50221: avoid speculation from CA slot · d382c5be
      Mauro Carvalho Chehab authored
      As warned by smatch:
      	drivers/media/dvb-core/dvb_ca_en50221.c:1392 dvb_ca_en50221_io_do_ioctl() warn: potential spectre issue 'ca->slot_info' [r] (local cap)
      
      There's a potential of using a CAM ioctl for speculation.
      
      The risk here is minimum, as only a small subset of DVB
      boards have CI, with a CAM module installed. Also, exploiting
      it would require a user capable of starting a DVB application.
      
      There are probably a lot of easier ways to try to exploit.
      
      Yet, it doesn't harm addressing it.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      d382c5be
  2. 16 Jun, 2021 4 commits
  3. 11 Jun, 2021 1 commit
    • Benjamin Drung's avatar
      media: uvcvideo: Fix pixel format change for Elgato Cam Link 4K · 4c6e0976
      Benjamin Drung authored
      The Elgato Cam Link 4K HDMI video capture card reports to support three
      different pixel formats, where the first format depends on the connected
      HDMI device.
      
      ```
      $ v4l2-ctl -d /dev/video0 --list-formats-ext
      ioctl: VIDIOC_ENUM_FMT
      	Type: Video Capture
      
      	[0]: 'NV12' (Y/CbCr 4:2:0)
      		Size: Discrete 3840x2160
      			Interval: Discrete 0.033s (29.970 fps)
      	[1]: 'NV12' (Y/CbCr 4:2:0)
      		Size: Discrete 3840x2160
      			Interval: Discrete 0.033s (29.970 fps)
      	[2]: 'YU12' (Planar YUV 4:2:0)
      		Size: Discrete 3840x2160
      			Interval: Discrete 0.033s (29.970 fps)
      ```
      
      Changing the pixel format to anything besides the first pixel format
      does not work:
      
      ```
      $ v4l2-ctl -d /dev/video0 --try-fmt-video pixelformat=YU12
      Format Video Capture:
      	Width/Height      : 3840/2160
      	Pixel Format      : 'NV12' (Y/CbCr 4:2:0)
      	Field             : None
      	Bytes per Line    : 3840
      	Size Image        : 12441600
      	Colorspace        : sRGB
      	Transfer Function : Rec. 709
      	YCbCr/HSV Encoding: Rec. 709
      	Quantization      : Default (maps to Limited Range)
      	Flags             :
      ```
      
      User space applications like VLC might show an error message on the
      terminal in that case:
      
      ```
      libv4l2: error set_fmt gave us a different result than try_fmt!
      ```
      
      Depending on the error handling of the user space applications, they
      might display a distorted video, because they use the wrong pixel format
      for decoding the stream.
      
      The Elgato Cam Link 4K responds to the USB video probe
      VS_PROBE_CONTROL/VS_COMMIT_CONTROL with a malformed data structure: The
      second byte contains bFormatIndex (instead of being the second byte of
      bmHint). The first byte is always zero. The third byte is always 1.
      
      The firmware bug was reported to Elgato on 2020-12-01 and it was
      forwarded by the support team to the developers as feature request.
      There is no firmware update available since then. The latest firmware
      for Elgato Cam Link 4K as of 2021-03-23 has MCU 20.02.19 and FPGA 67.
      
      Therefore correct the malformed data structure for this device. The
      change was successfully tested with VLC, OBS, and Chromium using
      different pixel formats (YUYV, NV12, YU12), resolutions (3840x2160,
      1920x1080), and frame rates (29.970 and 59.940 fps).
      
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarBenjamin Drung <bdrung@posteo.de>
      Signed-off-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      4c6e0976
  4. 09 Jun, 2021 1 commit
    • Mauro Carvalho Chehab's avatar
      media: dmxdev: change the check for problems allocing secfeed · 3d42c93e
      Mauro Carvalho Chehab authored
      While the logic there is right, it tricks static check analyzers,
      like smatch:
      
      	drivers/media/dvb-core/dmxdev.c:729 dvb_dmxdev_filter_start() error: we previously assumed '*secfeed' could be null (see line 719)
      
      Because the implementation of the filter itself is made via
      a callback, with its real implementation at the
      dvbdmx_allocate_section_feed() inside dvb_demux.c.
      
      So, change the check logic to make it clear that the function
      will not try to use *secfeed == NULL.
      Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
      3d42c93e
  5. 08 Jun, 2021 7 commits