Re: [PATCH v7 09/18] v4l: async: Move async subdev notifier operations to a separate structure

[Hans Verkuil]

On 09/03/2017 07:49 PM, Sakari Ailus wrote:
> From: Laurent Pinchart 
> 
> The async subdev notifier .bound(), .unbind() and .complete() operations
> are function pointers stored directly in the v4l2_async_subdev
> structure. As the structure isn't immutable, this creates a potential
> security risk as the function pointers are mutable.
> 
> To fix this, move the function pointers to a new
> v4l2_async_subdev_operations structure that can be made const in
> drivers.
> 
> Signed-off-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/am437x/am437x-vpfe.c|  8 +--
>  drivers/media/platform/atmel/atmel-isc.c   | 10 ++---
>  drivers/media/platform/atmel/atmel-isi.c   | 10 ++---
>  drivers/media/platform/davinci/vpif_capture.c  |  8 +--
>  drivers/media/platform/davinci/vpif_display.c  |  8 +--
>  drivers/media/platform/exynos4-is/media-dev.c  |  8 +--
>  drivers/media/platform/omap3isp/isp.c  |  6 +-
>  drivers/media/platform/pxa_camera.c|  8 +--
>  drivers/media/platform/qcom/camss-8x16/camss.c |  8 +--
>  drivers/media/platform/rcar-vin/rcar-core.c| 10 ++---
>  drivers/media/platform/rcar_drif.c | 10 ++---
>  drivers/media/platform/soc_camera/soc_camera.c | 14 +++--
>  drivers/media/platform/stm32/stm32-dcmi.c  | 10 ++---
>  drivers/media/platform/ti-vpe/cal.c|  8 +--
>  drivers/media/platform/xilinx/xilinx-vipp.c|  8 +--
>  drivers/media/v4l2-core/v4l2-async.c   | 20 +-
>  drivers/staging/media/imx/imx-media-dev.c  |  8 +--
>  include/media/v4l2-async.h | 29 
> +-
>  18 files changed, 131 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index dfcc484cab89..0997c640191d 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2417,6 +2417,11 @@ static int vpfe_async_complete(struct 
> v4l2_async_notifier *notifier)
>   return vpfe_probe_complete(vpfe);
>  }
>  
> +static const struct v4l2_async_notifier_operations vpfe_async_ops = {
> + .bound = vpfe_async_bound,
> + .complete = vpfe_async_complete,
> +};
> +
>  static struct vpfe_config *
>  vpfe_get_pdata(struct platform_device *pdev)
>  {
> @@ -2590,8 +2595,7 @@ static int vpfe_probe(struct platform_device *pdev)
>  
>   vpfe->notifier.subdevs = vpfe->cfg->asd;
>   vpfe->notifier.num_subdevs = ARRAY_SIZE(vpfe->cfg->asd);
> - vpfe->notifier.bound = vpfe_async_bound;
> - vpfe->notifier.complete = vpfe_async_complete;
> + vpfe->notifier.ops = &vpfe_async_ops;
>   ret = v4l2_async_notifier_register(&vpfe->v4l2_dev,
>   &vpfe->notifier);
>   if (ret) {
> diff --git a/drivers/media/platform/atmel/atmel-isc.c 
> b/drivers/media/platform/atmel/atmel-isc.c
> index d7103c5f92c3..48544c4137cb 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -1639,6 +1639,12 @@ static int isc_async_complete(struct 
> v4l2_async_notifier *notifier)
>   return 0;
>  }
>  
> +static const struct v4l2_async_notifier_operations isc_async_ops = {
> + .bound = isc_async_bound,
> + .unbind = isc_async_unbind,
> + .complete = isc_async_complete,
> +};
> +
>  static void isc_subdev_cleanup(struct isc_device *isc)
>  {
>   struct isc_subdev_entity *subdev_entity;
> @@ -1851,9 +1857,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
>   list_for_each_entry(subdev_entity, &isc->subdev_entities, list) {
>   subdev_entity->notifier.subdevs = &subdev_entity->asd;
>   subdev_entity->notifier.num_subdevs = 1;
> - subdev_entity->notifier.bound = isc_async_bound;
> - subdev_entity->notifier.unbind = isc_async_unbind;
> - subdev_entity->notifier.complete = isc_async_complete;
> + subdev_entity->notifier.ops = &isc_async_ops;
>  
>   ret = v4l2_async_notifier_register(&isc->v4l2_dev,
>  &subdev_entity->notifier);
> diff --git a/drivers/media/platform/atmel/atmel-isi.c 
> b/drivers/media/platform/atmel/atmel-isi.c
> index 891fa2505efa..eadbf9def358 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1105,6 +1105,12 @@ static int isi_graph_notify_bound(struct 
> v4l2_async_notifier *notifier,
>   return 0;
>  }
>  
> +static const struct v4l2_async_notifier_operations isi_graph_notify_ops = {
> + .bound = isi_graph_notify_bound,
> + .unbind = isi_graph_notify_unbind,
> + .complete = isi_graph_notify_complete,
> +};
> +
>  static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
>  {
>   struct devic

[PATCH v7 09/18] v4l: async: Move async subdev notifier operations to a separate structure

[Sakari Ailus]

From: Laurent Pinchart 

The async subdev notifier .bound(), .unbind() and .complete() operations
are function pointers stored directly in the v4l2_async_subdev
structure. As the structure isn't immutable, this creates a potential
security risk as the function pointers are mutable.

To fix this, move the function pointers to a new
v4l2_async_subdev_operations structure that can be made const in
drivers.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/am437x/am437x-vpfe.c|  8 +--
 drivers/media/platform/atmel/atmel-isc.c   | 10 ++---
 drivers/media/platform/atmel/atmel-isi.c   | 10 ++---
 drivers/media/platform/davinci/vpif_capture.c  |  8 +--
 drivers/media/platform/davinci/vpif_display.c  |  8 +--
 drivers/media/platform/exynos4-is/media-dev.c  |  8 +--
 drivers/media/platform/omap3isp/isp.c  |  6 +-
 drivers/media/platform/pxa_camera.c|  8 +--
 drivers/media/platform/qcom/camss-8x16/camss.c |  8 +--
 drivers/media/platform/rcar-vin/rcar-core.c| 10 ++---
 drivers/media/platform/rcar_drif.c | 10 ++---
 drivers/media/platform/soc_camera/soc_camera.c | 14 +++--
 drivers/media/platform/stm32/stm32-dcmi.c  | 10 ++---
 drivers/media/platform/ti-vpe/cal.c|  8 +--
 drivers/media/platform/xilinx/xilinx-vipp.c|  8 +--
 drivers/media/v4l2-core/v4l2-async.c   | 20 +-
 drivers/staging/media/imx/imx-media-dev.c  |  8 +--
 include/media/v4l2-async.h | 29 +-
 18 files changed, 131 insertions(+), 60 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index dfcc484cab89..0997c640191d 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2417,6 +2417,11 @@ static int vpfe_async_complete(struct 
v4l2_async_notifier *notifier)
return vpfe_probe_complete(vpfe);
 }
 
+static const struct v4l2_async_notifier_operations vpfe_async_ops = {
+   .bound = vpfe_async_bound,
+   .complete = vpfe_async_complete,
+};
+
 static struct vpfe_config *
 vpfe_get_pdata(struct platform_device *pdev)
 {
@@ -2590,8 +2595,7 @@ static int vpfe_probe(struct platform_device *pdev)
 
vpfe->notifier.subdevs = vpfe->cfg->asd;
vpfe->notifier.num_subdevs = ARRAY_SIZE(vpfe->cfg->asd);
-   vpfe->notifier.bound = vpfe_async_bound;
-   vpfe->notifier.complete = vpfe_async_complete;
+   vpfe->notifier.ops = &vpfe_async_ops;
ret = v4l2_async_notifier_register(&vpfe->v4l2_dev,
&vpfe->notifier);
if (ret) {
diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index d7103c5f92c3..48544c4137cb 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1639,6 +1639,12 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
return 0;
 }
 
+static const struct v4l2_async_notifier_operations isc_async_ops = {
+   .bound = isc_async_bound,
+   .unbind = isc_async_unbind,
+   .complete = isc_async_complete,
+};
+
 static void isc_subdev_cleanup(struct isc_device *isc)
 {
struct isc_subdev_entity *subdev_entity;
@@ -1851,9 +1857,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
list_for_each_entry(subdev_entity, &isc->subdev_entities, list) {
subdev_entity->notifier.subdevs = &subdev_entity->asd;
subdev_entity->notifier.num_subdevs = 1;
-   subdev_entity->notifier.bound = isc_async_bound;
-   subdev_entity->notifier.unbind = isc_async_unbind;
-   subdev_entity->notifier.complete = isc_async_complete;
+   subdev_entity->notifier.ops = &isc_async_ops;
 
ret = v4l2_async_notifier_register(&isc->v4l2_dev,
   &subdev_entity->notifier);
diff --git a/drivers/media/platform/atmel/atmel-isi.c 
b/drivers/media/platform/atmel/atmel-isi.c
index 891fa2505efa..eadbf9def358 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1105,6 +1105,12 @@ static int isi_graph_notify_bound(struct 
v4l2_async_notifier *notifier,
return 0;
 }
 
+static const struct v4l2_async_notifier_operations isi_graph_notify_ops = {
+   .bound = isi_graph_notify_bound,
+   .unbind = isi_graph_notify_unbind,
+   .complete = isi_graph_notify_complete,
+};
+
 static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
 {
struct device_node *ep = NULL;
@@ -1152,9 +1158,7 @@ static int isi_graph_init(struct atmel_isi *isi)
 
isi->notifier.subdevs = subdevs;
isi->notifier.num_subdevs = 1;
-   isi->notifier.bound = isi_graph_notify_bound;
-   isi->notifier.unbind =