diff mbox series

[v2,02/15] media: ipu-bridge: Do not use on stack memory for software_node.name field

Message ID 20230630110643.209761-3-hdegoede@redhat.com
State Superseded
Headers show
Series media: ipu-bridge: Shared with atomisp, rework VCM instantiation | expand

Commit Message

Hans de Goede June 30, 2023, 11:06 a.m. UTC
Commit 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs
with same HID") introduced an on stack vcm_name and then uses this for
the name field of the software_node struct used for the vcm.

But the software_node struct is much longer lived then the current
stack-frame, so this is no good.

Instead extend the ipu_node_names struct with an extra field to store
the vcm software_node name and use that.

Note this also changes the length of the allocated buffer from
ACPI_ID_LEN + 4 to 16. the name is filled with "<ipu_vcm_types[x]>-%u"
where ipu_vcm_types[x] is not an ACPI_ID. The maximum length of
the strings in the ipu_vcm_types[] array is 11 + 5 bytes for "-255\0"
means 16 bytes are needed in the worst case scenario.

Fixes: 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs with same HID")
Cc: Bingbu Cao <bingbu.cao@intel.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/pci/intel/ipu-bridge.c | 7 +++----
 drivers/media/pci/intel/ipu-bridge.h | 1 +
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Dan Scally July 4, 2023, 11:03 a.m. UTC | #1
Hi Hans

On 30/06/2023 13:06, Hans de Goede wrote:
> Commit 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs
> with same HID") introduced an on stack vcm_name and then uses this for
> the name field of the software_node struct used for the vcm.
>
> But the software_node struct is much longer lived then the current
> stack-frame, so this is no good.
>
> Instead extend the ipu_node_names struct with an extra field to store
> the vcm software_node name and use that.
>
> Note this also changes the length of the allocated buffer from
> ACPI_ID_LEN + 4 to 16. the name is filled with "<ipu_vcm_types[x]>-%u"
> where ipu_vcm_types[x] is not an ACPI_ID. The maximum length of
> the strings in the ipu_vcm_types[] array is 11 + 5 bytes for "-255\0"
> means 16 bytes are needed in the worst case scenario.
>
> Fixes: 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs with same HID")
> Cc: Bingbu Cao <bingbu.cao@intel.com>
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---


Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/media/pci/intel/ipu-bridge.c | 7 +++----
>   drivers/media/pci/intel/ipu-bridge.h | 1 +
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index f0927f80184d..ef6c6cb7b51b 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -220,7 +220,6 @@ static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge,
>   						 struct ipu_sensor *sensor)
>   {
>   	struct software_node *nodes = sensor->swnodes;
> -	char vcm_name[ACPI_ID_LEN + 4];
>   
>   	ipu_bridge_init_swnode_names(sensor);
>   
> @@ -240,10 +239,10 @@ static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge,
>   						sensor->ipu_properties);
>   	if (sensor->ssdb.vcmtype) {
>   		/* append ssdb.link to distinguish VCM nodes with same HID */
> -		snprintf(vcm_name, sizeof(vcm_name), "%s-%u",
> -			 ipu_vcm_types[sensor->ssdb.vcmtype - 1],
> +		snprintf(sensor->node_names.vcm, sizeof(sensor->node_names.vcm),
> +			 "%s-%u", ipu_vcm_types[sensor->ssdb.vcmtype - 1],
>   			 sensor->ssdb.link);
> -		nodes[SWNODE_VCM] = NODE_VCM(vcm_name);
> +		nodes[SWNODE_VCM] = NODE_VCM(sensor->node_names.vcm);
>   	}
>   
>   	ipu_bridge_init_swnode_group(sensor);
> diff --git a/drivers/media/pci/intel/ipu-bridge.h b/drivers/media/pci/intel/ipu-bridge.h
> index 8cb733c03e2f..6cce712a0f34 100644
> --- a/drivers/media/pci/intel/ipu-bridge.h
> +++ b/drivers/media/pci/intel/ipu-bridge.h
> @@ -103,6 +103,7 @@ struct ipu_node_names {
>   	char port[7];
>   	char endpoint[11];
>   	char remote_port[7];
> +	char vcm[16];
>   };
>   
>   struct ipu_sensor_config {
diff mbox series

Patch

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index f0927f80184d..ef6c6cb7b51b 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -220,7 +220,6 @@  static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge,
 						 struct ipu_sensor *sensor)
 {
 	struct software_node *nodes = sensor->swnodes;
-	char vcm_name[ACPI_ID_LEN + 4];
 
 	ipu_bridge_init_swnode_names(sensor);
 
@@ -240,10 +239,10 @@  static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge,
 						sensor->ipu_properties);
 	if (sensor->ssdb.vcmtype) {
 		/* append ssdb.link to distinguish VCM nodes with same HID */
-		snprintf(vcm_name, sizeof(vcm_name), "%s-%u",
-			 ipu_vcm_types[sensor->ssdb.vcmtype - 1],
+		snprintf(sensor->node_names.vcm, sizeof(sensor->node_names.vcm),
+			 "%s-%u", ipu_vcm_types[sensor->ssdb.vcmtype - 1],
 			 sensor->ssdb.link);
-		nodes[SWNODE_VCM] = NODE_VCM(vcm_name);
+		nodes[SWNODE_VCM] = NODE_VCM(sensor->node_names.vcm);
 	}
 
 	ipu_bridge_init_swnode_group(sensor);
diff --git a/drivers/media/pci/intel/ipu-bridge.h b/drivers/media/pci/intel/ipu-bridge.h
index 8cb733c03e2f..6cce712a0f34 100644
--- a/drivers/media/pci/intel/ipu-bridge.h
+++ b/drivers/media/pci/intel/ipu-bridge.h
@@ -103,6 +103,7 @@  struct ipu_node_names {
 	char port[7];
 	char endpoint[11];
 	char remote_port[7];
+	char vcm[16];
 };
 
 struct ipu_sensor_config {