linux-kernel - Re: [PATCH v4 2/2] media: qcom: camss: Restructure camss_link

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] [day] [month] [year] [list]

Message-ID: <1a570c17-c501-4a29-a4f7-020f41563f3d@linaro.org>
Date: Sat, 14 Dec 2024 00:44:38 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Vikram Sharma <quic_vikramsa@...cinc.com>, rfoss@...nel.org,
 todor.too@...il.com
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 kernel@...cinc.com
Subject: Re: [PATCH v4 2/2] media: qcom: camss: Restructure
 camss_link_entities

On 26/11/2024 10:01, Vikram Sharma wrote:
> Refactor the camss_link_entities function by breaking it down into
> three distinct functions. Each function will handle the linking of
> a specific entity separately.
> SC7280 and later targets mandates for 1:1 linking for csid -> vfe.
> i.e. csid0 can be mapped to vfe0 only.
> 
> Signed-off-by: Suresh Vankadara <quic_svankada@...cinc.com>
> Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@...cinc.com>
> Signed-off-by: Vikram Sharma <quic_vikramsa@...cinc.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> ---
>   drivers/media/platform/qcom/camss/camss.c | 155 ++++++++++++++--------
>   1 file changed, 103 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 6824ffbdf4a8..67fb11cbe865 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1994,7 +1994,6 @@ static int camss_init_subdevices(struct camss *camss)
>   }
>   
>   /*
> - * camss_link_entities - Register subdev nodes and create links
>    * camss_link_err - print error in case link creation fails
>    * @src_name: name for source of the link
>    * @sink_name: name for sink of the link
> @@ -2012,14 +2011,64 @@ inline void camss_link_err(struct camss *camss,
>   }
>   
>   /*
> - * camss_link_entities - Register subdev nodes and create links
> + * camss_link_entities_csid - Register subdev nodes and create links
>    * @camss: CAMSS device
>    *
>    * Return 0 on success or a negative error code on failure
>    */
> -static int camss_link_entities(struct camss *camss)
> +static int camss_link_entities_csid(struct camss *camss)
>   {
> -	int i, j, k;
> +	struct media_entity *src_entity;
> +	struct media_entity *sink_entity;
> +	int ret, line_num;
> +	u16 sink_pad;
> +	u16 src_pad;
> +	int i, j;
> +
> +	for (i = 0; i < camss->res->csid_num; i++) {
> +		if (camss->ispif)
> +			line_num = camss->ispif->line_num;
> +		else
> +			line_num = camss->vfe[i].res->line_num;

This statement is incorrect, you are indexing vfe[] with a control 
derived from csid_num.

Below is the statement you are removing.

> -		for (i = 0; i < camss->res->csid_num; i++)
> -			for (k = 0; k < camss->res->vfe_num; k++)
> -				for (j = 0; j < camss->vfe[k].res->line_num; j++) {

As soon as csid_num > ARRAY_SIZE(vfe) your code breaks.

I noticed this as I added csid_lite to the x1e series where the 
aggregate count of CSID is grater than the aggregate count of VFE.

Please fix.

---
bod