linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity
@ 2019-08-29 11:00 Dafna Hirschfeld
  2019-08-29 13:06 ` André Almeida
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Dafna Hirschfeld @ 2019-08-29 11:00 UTC (permalink / raw)
  To: linux-media
  Cc: dafna.hirschfeld, laurent.pinchart, helen.koike, ezequiel, andre.almeida

Userspace can disable links and create pipelines that
do not start with a source entity. Trying to stream
from such a pipeline should fail with -EPIPE
currently this is not handled and cause kernel crash.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
Hi,
These are the commands to reproduce the crash:
media-ctl -d0 -l "5:1->21:0[0]" -v
v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2

drivers/media/platform/vimc/vimc-streamer.c | 39 +++++++++++++++------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index 048d770e498b..bfc7921c7a8b 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -13,6 +13,19 @@
 
 #include "vimc-streamer.h"
 
+/**
+ * Check if the entity has only source pads
+ */
+static bool vimc_is_source(struct media_entity *ent)
+{
+	int i;
+
+	for (i = 0; i < ent->num_pads; i++)
+		if (ent->pads[i].flags & MEDIA_PAD_FL_SINK)
+			return false;
+	return true;
+}
+
 /**
  * vimc_get_source_entity - get the entity connected with the first sink pad
  *
@@ -83,14 +96,12 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 	struct media_entity *entity;
 	struct video_device *vdev;
 	struct v4l2_subdev *sd;
-	int ret = 0;
+	int ret = -EINVAL;
 
 	stream->pipe_size = 0;
 	while (stream->pipe_size < VIMC_STREAMER_PIPELINE_MAX_SIZE) {
-		if (!ved) {
-			vimc_streamer_pipeline_terminate(stream);
-			return -EINVAL;
-		}
+		if (!ved)
+			break;
 		stream->ved_pipeline[stream->pipe_size++] = ved;
 
 		if (is_media_entity_v4l2_subdev(ved->ent)) {
@@ -99,15 +110,22 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 			if (ret && ret != -ENOIOCTLCMD) {
 				pr_err("subdev_call error %s\n",
 				       ved->ent->name);
-				vimc_streamer_pipeline_terminate(stream);
-				return ret;
+				break;
 			}
 		}
 
 		entity = vimc_get_source_entity(ved->ent);
-		/* Check if the end of the pipeline was reached*/
-		if (!entity)
+		/* Check if the end of the pipeline was reached */
+		if (!entity) {
+			/* the first entity of the pipe should be source only */
+			if (!vimc_is_source(ved->ent)) {
+				pr_err("first entity in the pipe '%s' is not a source\n",
+				       ved->ent->name);
+				ret = -EPIPE;
+				break;
+			}
 			return 0;
+		}
 
 		/* Get the next device in the pipeline */
 		if (is_media_entity_v4l2_subdev(entity)) {
@@ -120,9 +138,8 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 			ved = video_get_drvdata(vdev);
 		}
 	}
-
 	vimc_streamer_pipeline_terminate(stream);
-	return -EINVAL;
+	return ret;
 }
 
 /**
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity
  2019-08-29 11:00 [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity Dafna Hirschfeld
@ 2019-08-29 13:06 ` André Almeida
  2019-08-29 19:44 ` kbuild test robot
  2019-08-30 12:57 ` Helen Koike
  2 siblings, 0 replies; 4+ messages in thread
From: André Almeida @ 2019-08-29 13:06 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: laurent.pinchart, helen.koike, ezequiel

Hello Dafna,

Thanks for your patch!

On 8/29/19 8:00 AM, Dafna Hirschfeld wrote:
> Userspace can disable links and create pipelines that
> do not start with a source entity. Trying to stream
> from such a pipeline should fail with -EPIPE
> currently this is not handled and cause kernel crash.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> Hi,
> These are the commands to reproduce the crash:
> media-ctl -d0 -l "5:1->21:0[0]" -v
> v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
> v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2
>
> drivers/media/platform/vimc/vimc-streamer.c | 39 +++++++++++++++------
>  1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
> index 048d770e498b..bfc7921c7a8b 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -13,6 +13,19 @@
>  
>  #include "vimc-streamer.h"
>  
> +/**
> + * Check if the entity has only source pads
> + */

You should only use /** when you are writing kernel-doc comments. You
may either replace /** by /*, or add a function documentation[1].

Thanks,
    André

[1]
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

> +static bool vimc_is_source(struct media_entity *ent)
> +{
> +	int i;
> +
> +	for (i = 0; i < ent->num_pads; i++)
> +		if (ent->pads[i].flags & MEDIA_PAD_FL_SINK)
> +			return false;
> +	return true;
> +}
> +
>  /**
>   * vimc_get_source_entity - get the entity connected with the first sink pad
>   *
> @@ -83,14 +96,12 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  	struct media_entity *entity;
>  	struct video_device *vdev;
>  	struct v4l2_subdev *sd;
> -	int ret = 0;
> +	int ret = -EINVAL;
>  
>  	stream->pipe_size = 0;
>  	while (stream->pipe_size < VIMC_STREAMER_PIPELINE_MAX_SIZE) {
> -		if (!ved) {
> -			vimc_streamer_pipeline_terminate(stream);
> -			return -EINVAL;
> -		}
> +		if (!ved)
> +			break;
>  		stream->ved_pipeline[stream->pipe_size++] = ved;
>  
>  		if (is_media_entity_v4l2_subdev(ved->ent)) {
> @@ -99,15 +110,22 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  			if (ret && ret != -ENOIOCTLCMD) {
>  				pr_err("subdev_call error %s\n",
>  				       ved->ent->name);
> -				vimc_streamer_pipeline_terminate(stream);
> -				return ret;
> +				break;
>  			}
>  		}
>  
>  		entity = vimc_get_source_entity(ved->ent);
> -		/* Check if the end of the pipeline was reached*/
> -		if (!entity)
> +		/* Check if the end of the pipeline was reached */
> +		if (!entity) {
> +			/* the first entity of the pipe should be source only */
> +			if (!vimc_is_source(ved->ent)) {
> +				pr_err("first entity in the pipe '%s' is not a source\n",
> +				       ved->ent->name);
> +				ret = -EPIPE;
> +				break;
> +			}
>  			return 0;
> +		}
>  
>  		/* Get the next device in the pipeline */
>  		if (is_media_entity_v4l2_subdev(entity)) {
> @@ -120,9 +138,8 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  			ved = video_get_drvdata(vdev);
>  		}
>  	}
> -
>  	vimc_streamer_pipeline_terminate(stream);
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  /**

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity
  2019-08-29 11:00 [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity Dafna Hirschfeld
  2019-08-29 13:06 ` André Almeida
@ 2019-08-29 19:44 ` kbuild test robot
  2019-08-30 12:57 ` Helen Koike
  2 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2019-08-29 19:44 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: kbuild-all, linux-media, dafna.hirschfeld, laurent.pinchart,
	helen.koike, ezequiel, andre.almeida

[-- Attachment #1: Type: text/plain, Size: 14517 bytes --]

Hi Dafna,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[cannot apply to v5.3-rc6 next-20190829]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dafna-Hirschfeld/media-vimc-upon-streaming-check-that-the-pipeline-starts-with-a-source-entity/20190830-013848
base:   git://linuxtv.org/media_tree.git master
reproduce: make htmldocs

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   Warning: The Sphinx 'sphinx_rtd_theme' HTML theme was not found. Make sure you have the theme installed to produce pretty HTML output. Falling back to the default theme.
   WARNING: dot(1) not found, for better output quality install graphviz from http://www.graphviz.org
   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
   include/linux/w1.h:272: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quotactl' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quota_on' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_free_mnt_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_eat_lsm_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_kern_mount' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_show_options' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_add_mnt_opt' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'd_instantiate' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'getprocattr' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'setprocattr' not described in 'security_list_options'
>> drivers/media/platform/vimc/vimc-streamer.c:20: warning: Function parameter or member 'ent' not described in 'vimc_is_source'
   lib/genalloc.c:1: warning: 'gen_pool_add_virt' not found
   lib/genalloc.c:1: warning: 'gen_pool_alloc' not found
   lib/genalloc.c:1: warning: 'gen_pool_free' not found
   lib/genalloc.c:1: warning: 'gen_pool_alloc_algo' not found
   include/linux/i2c.h:337: warning: Function parameter or member 'init_irq' not described in 'i2c_client'
   include/linux/regulator/machine.h:196: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints'
   include/linux/regulator/driver.h:223: warning: Function parameter or member 'resume' not described in 'regulator_ops'
   include/linux/spi/spi.h:190: warning: Function parameter or member 'driver_override' not described in 'spi_device'
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not found
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_unregister_notifier' not found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not found
   include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry'
   fs/direct-io.c:258: warning: Excess function parameter 'offset' description in 'dio_complete'
   fs/libfs.c:496: warning: Excess function parameter 'available' description in 'simple_write_end'
   fs/posix_acl.c:647: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:647: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:647: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
   include/linux/skbuff.h:893: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'vlan_present' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:233: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:515: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock'
   include/net/sock.h:2439: warning: Function parameter or member 'tcp_rx_skb_cache_key' not described in 'DECLARE_STATIC_KEY_FALSE'
   include/net/sock.h:2439: warning: Excess function parameter 'sk' description in 'DECLARE_STATIC_KEY_FALSE'
   include/net/sock.h:2439: warning: Excess function parameter 'skb' description in 'DECLARE_STATIC_KEY_FALSE'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
   drivers/net/phy/phylink.c:595: warning: Function parameter or member 'config' not described in 'phylink_create'
   drivers/net/phy/phylink.c:595: warning: Excess function parameter 'ndev' description in 'phylink_create'
   include/net/mac80211.h:2006: warning: Function parameter or member 'txpwr' not described in 'ieee80211_sta'
   mm/util.c:1: warning: 'get_user_pages_fast' not found
   mm/slab.c:4215: warning: Function parameter or member 'objp' not described in '__ksize'
   include/net/cfg80211.h:1092: warning: Function parameter or member 'txpwr' not described in 'station_parameters'
   include/net/mac80211.h:4043: warning: Function parameter or member 'sta_set_txpwr' not described in 'ieee80211_ops'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:142: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:347: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:348: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:494: warning: Function parameter or member 'start' not described in 'amdgpu_vm_pt_first_dfs'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:823: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_flags'

vim +20 drivers/media/platform/vimc/vimc-streamer.c

    15	
    16	/**
    17	 * Check if the entity has only source pads
    18	 */
    19	static bool vimc_is_source(struct media_entity *ent)
  > 20	{
    21		int i;
    22	
    23		for (i = 0; i < ent->num_pads; i++)
    24			if (ent->pads[i].flags & MEDIA_PAD_FL_SINK)
    25				return false;
    26		return true;
    27	}
    28	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7282 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity
  2019-08-29 11:00 [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity Dafna Hirschfeld
  2019-08-29 13:06 ` André Almeida
  2019-08-29 19:44 ` kbuild test robot
@ 2019-08-30 12:57 ` Helen Koike
  2 siblings, 0 replies; 4+ messages in thread
From: Helen Koike @ 2019-08-30 12:57 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media
  Cc: laurent.pinchart, helen.koike, ezequiel, andre.almeida

Hi Dafna,

Thanks for the patch. Just small comments below.

On 8/29/19 8:00 AM, Dafna Hirschfeld wrote:
> Userspace can disable links and create pipelines that
> do not start with a source entity. Trying to stream
> from such a pipeline should fail with -EPIPE
> currently this is not handled and cause kernel crash.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> Hi,
> These are the commands to reproduce the crash:
> media-ctl -d0 -l "5:1->21:0[0]" -v
> v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
> v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2
> 
> drivers/media/platform/vimc/vimc-streamer.c | 39 +++++++++++++++------
>  1 file changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
> index 048d770e498b..bfc7921c7a8b 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -13,6 +13,19 @@
>  
>  #include "vimc-streamer.h"
>  
> +/**
> + * Check if the entity has only source pads
> + */

please add docs like the other functions in this file.

> +static bool vimc_is_source(struct media_entity *ent)
> +{
> +	int i;

unsigned

Regards,
Helen

> +
> +	for (i = 0; i < ent->num_pads; i++)
> +		if (ent->pads[i].flags & MEDIA_PAD_FL_SINK)
> +			return false;
> +	return true;
> +}
> +
>  /**
>   * vimc_get_source_entity - get the entity connected with the first sink pad
>   *
> @@ -83,14 +96,12 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  	struct media_entity *entity;
>  	struct video_device *vdev;
>  	struct v4l2_subdev *sd;
> -	int ret = 0;
> +	int ret = -EINVAL;
>  
>  	stream->pipe_size = 0;
>  	while (stream->pipe_size < VIMC_STREAMER_PIPELINE_MAX_SIZE) {
> -		if (!ved) {
> -			vimc_streamer_pipeline_terminate(stream);
> -			return -EINVAL;
> -		}
> +		if (!ved)
> +			break;
>  		stream->ved_pipeline[stream->pipe_size++] = ved;
>  
>  		if (is_media_entity_v4l2_subdev(ved->ent)) {
> @@ -99,15 +110,22 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  			if (ret && ret != -ENOIOCTLCMD) {
>  				pr_err("subdev_call error %s\n",
>  				       ved->ent->name);
> -				vimc_streamer_pipeline_terminate(stream);
> -				return ret;
> +				break;
>  			}
>  		}
>  
>  		entity = vimc_get_source_entity(ved->ent);
> -		/* Check if the end of the pipeline was reached*/
> -		if (!entity)
> +		/* Check if the end of the pipeline was reached */
> +		if (!entity) {
> +			/* the first entity of the pipe should be source only */
> +			if (!vimc_is_source(ved->ent)) {
> +				pr_err("first entity in the pipe '%s' is not a source\n",
> +				       ved->ent->name);
> +				ret = -EPIPE;
> +				break;
> +			}
>  			return 0;
> +		}
>  
>  		/* Get the next device in the pipeline */
>  		if (is_media_entity_v4l2_subdev(entity)) {
> @@ -120,9 +138,8 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  			ved = video_get_drvdata(vdev);
>  		}
>  	}
> -
>  	vimc_streamer_pipeline_terminate(stream);
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  /**
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-08-30 12:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 11:00 [PATCH] media: vimc: upon streaming, check that the pipeline starts with a source entity Dafna Hirschfeld
2019-08-29 13:06 ` André Almeida
2019-08-29 19:44 ` kbuild test robot
2019-08-30 12:57 ` Helen Koike

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).