Re: [PATCH v7 03/10] ACPI: property: Parse _CRS CSI-2 descriptor

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-acpi@vger.kernel.org, linux-media@vger.kernel.org,
	rafael@kernel.org, heikki.krogerus@linux.intel.com
Subject: Re: [PATCH v7 03/10] ACPI: property: Parse _CRS CSI-2 descriptor
Date: Tue, 28 Mar 2023 18:12:09 +0300	[thread overview]
Message-ID: <ZCMDyVoFWizx+tOp@smile.fi.intel.com> (raw)
In-Reply-To: <20230328101303.1458570-4-sakari.ailus@linux.intel.com>

On Tue, Mar 28, 2023 at 01:12:56PM +0300, Sakari Ailus wrote:
> Parse newly added ACPI _CRS CSI-2 descriptor for CSI-2 and camera
> configuration, associate it with appropriate devices and allocate memory for
> software nodes needed to create a DT-like data structure for drivers.

...

> +struct acpi_scan_context {
> +	struct acpi_device *device;
> +	struct list_head postponed_head;

Make it first?

> +	struct acpi_scan_context_csi2 csi2;
> +};

...

> +void acpi_bus_scan_check_crs_csi2(acpi_handle handle, struct acpi_scan_context *ctx)
> +{
> +	struct scan_check_crs_csi2_context inst_context = {
> +		.handle = handle,
> +		.res_head = LIST_HEAD_INIT(inst_context.res_head),
> +	};
> +	struct crs_csi2 *csi2;
> +
> +	acpi_walk_resources(handle, METHOD_NAME__CRS,
> +			    scan_check_crs_csi2_instance, &inst_context);
> +
> +	if (list_empty(&inst_context.res_head))
> +		return;
> +
> +	/*
> +	 * Found entry, so allocate memory for it, fill it and add it to the
> +	 * list.
> +	 */
> +	csi2 = kmalloc(sizeof(*csi2), GFP_KERNEL);
> +	if (!csi2)

Who is going to release resources allocated in the callback above?

> +		return; /* There's nothing we really can do about this. */
> +
> +	csi2->handle = handle;
> +	list_replace(&inst_context.res_head, &csi2->buses);
> +	list_add(&csi2->list, &ctx->csi2.crs_csi2_head);
> +
> +	/* This handle plus remote handles in _CRS CSI2 resource descriptors */
> +	ctx->csi2.handle_count += 1 + inst_context.handle_count;
> +}

...

> +	/*
> +	 * Allocate memory for ports, node pointers (number of nodes +
> +	 * 1 (guardian), nodes (root + number of ports * 2 (for for
> +	 * every port there is an endpoint)).
> +	 */

for for ?

I am a bit lost here. Can you put the above in more mathematical language?

> +	overflow = check_mul_overflow(sizeof(*ads->ports) +
> +				      sizeof(*ads->nodes) * 2 +
> +				      sizeof(*ads->nodeptrs) * 2,
> +				      ports_count, &alloc_size);
> +	overflow = overflow ||
> +		   check_add_overflow(sizeof(*ads) + sizeof(*ads->nodes) +
> +				      sizeof(*ads->nodeptrs) * 2,
> +				      alloc_size, &alloc_size);
> +	if (overflow) {
> +		acpi_handle_warn(handle,
> +				 "too many _CRS CSI2 resource handles (%zu)",
> +				 ports_count);
> +		return;
> +	}

-- 
With Best Regards,
Andy Shevchenko